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鈥檒l 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 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,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))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,18 @@

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;
Expand All @@ -31,27 +35,60 @@ 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 =
extractSecurityCookieNames(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> 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,14 +2,18 @@

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;
Expand All @@ -31,27 +35,60 @@ 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 =
extractSecurityCookieNames(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> 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
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import org.springframework.mock.web.MockServletContext
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders
import java.net.URI
import javax.servlet.FilterChain
import javax.servlet.ServletContext
import javax.servlet.http.HttpServletRequest
import kotlin.test.Test
import kotlin.test.assertEquals
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 }
}
}
6 changes: 6 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -3751,8 +3751,13 @@ public abstract interface class io/sentry/util/HintUtils$SentryNullableConsumer
}

public final class io/sentry/util/HttpUtils {
public static final field COOKIE_HEADER_NAME Ljava/lang/String;
public fun <init> ()V
public static fun containsSensitiveHeader (Ljava/lang/String;)Z
public static fun filterOutSecurityCookies (Ljava/lang/String;Ljava/util/List;)Ljava/lang/String;
public static fun filterOutSecurityCookiesFromHeader (Ljava/util/Enumeration;Ljava/lang/String;Ljava/util/List;)Ljava/util/List;
public static fun filterOutSecurityCookiesFromHeader (Ljava/util/List;Ljava/lang/String;Ljava/util/List;)Ljava/util/List;
public static fun isSecurityCookie (Ljava/lang/String;Ljava/util/List;)Z
}

public final class io/sentry/util/JsonSerializationUtils {
Expand Down Expand Up @@ -3811,6 +3816,7 @@ public final class io/sentry/util/StringUtils {
}

public final class io/sentry/util/UrlUtils {
public static final field SENSITIVE_DATA_SUBSTITUTE Ljava/lang/String;
public fun <init> ()V
public static fun parse (Ljava/lang/String;)Lio/sentry/util/UrlUtils$UrlDetails;
public static fun parseNullable (Ljava/lang/String;)Lio/sentry/util/UrlUtils$UrlDetails;
Expand Down