Skip to content

Commit

Permalink
Filter out session cookies sent by Spring and Spring Boot integrations (
Browse files Browse the repository at this point in the history 
#2593)
  • Loading branch information
@adinauer
adinauer committed Mar 14, 2023
1 parent 25e36a7 commit 674b462
Show file tree
Hide file tree
Showing 11 changed files with 359 additions and 33 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
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
65 changes: 58 additions & 7 deletions sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryRequestResolver.java
View file
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) {
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;
}
}
17 changes: 13 additions & 4 deletions ...-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryRequestResolver.java
View file
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
18 changes: 13 additions & 5 deletions sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/SentrySpringFilterTest.kt
View file
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 }
}
}
65 changes: 58 additions & 7 deletions sentry-spring/src/main/java/io/sentry/spring/SentryRequestResolver.java
View file
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;
}
}
17 changes: 13 additions & 4 deletions sentry-spring/src/main/java/io/sentry/spring/webflux/SentryRequestResolver.java
View file
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

0 comments on commit 674b462

Please sign in to comment.