From 820a4faec29032eb4eb6427b0ea9ca01202a4f83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois-Xavier=20Thomas?= Date: Mon, 22 Apr 2019 01:07:00 +0200 Subject: [PATCH] Avoid logging sensitive URL parameters in the Subsonic API In case of exceptions, Airsonic logs the full URL that triggered it since 417583cc, including possibly sensitive query parameters such as the authentication password/tokens passed to the Subsonic API. This replaces the value set for this parameter in the URL by the "" string. --- .../player/controller/StreamController.java | 2 +- .../spring/LoggingExceptionResolver.java | 4 ++-- .../java/org/airsonic/player/util/Util.java | 24 +++++++++++++++++++ 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/airsonic-main/src/main/java/org/airsonic/player/controller/StreamController.java b/airsonic-main/src/main/java/org/airsonic/player/controller/StreamController.java index 83f0e6c7..dd026721 100644 --- a/airsonic-main/src/main/java/org/airsonic/player/controller/StreamController.java +++ b/airsonic-main/src/main/java/org/airsonic/player/controller/StreamController.java @@ -257,7 +257,7 @@ public class StreamController { shouldCatch |= Util.isInstanceOfClassName(e, "org.apache.catalina.connector.ClientAbortException"); shouldCatch |= Util.isInstanceOfClassName(e, "org.eclipse.jetty.io.EofException"); if (shouldCatch) { - LOG.info("{}: Client unexpectedly closed connection while loading {} ({})", request.getRemoteAddr(), Util.getURLForRequest(request), e.getCause().toString()); + LOG.info("{}: Client unexpectedly closed connection while loading {} ({})", request.getRemoteAddr(), Util.getAnonymizedURLForRequest(request), e.getCause().toString()); return; } diff --git a/airsonic-main/src/main/java/org/airsonic/player/spring/LoggingExceptionResolver.java b/airsonic-main/src/main/java/org/airsonic/player/spring/LoggingExceptionResolver.java index 0dcb193d..454a582e 100644 --- a/airsonic-main/src/main/java/org/airsonic/player/spring/LoggingExceptionResolver.java +++ b/airsonic-main/src/main/java/org/airsonic/player/spring/LoggingExceptionResolver.java @@ -25,12 +25,12 @@ public class LoggingExceptionResolver implements HandlerExceptionResolver, Order shouldCatch |= Util.isInstanceOfClassName(e, "org.apache.catalina.connector.ClientAbortException"); shouldCatch |= Util.isInstanceOfClassName(e, "org.eclipse.jetty.io.EofException"); if (shouldCatch) { - LOG.info("{}: Client unexpectedly closed connection while loading {} ({})", request.getRemoteAddr(), Util.getURLForRequest(request), e.getCause().toString()); + LOG.info("{}: Client unexpectedly closed connection while loading {} ({})", request.getRemoteAddr(), Util.getAnonymizedURLForRequest(request), e.getCause().toString()); return null; } // Display a full stack trace in all other cases - LOG.error("{}: An exception occurred while loading {}", request.getRemoteAddr(), Util.getURLForRequest(request), e); + LOG.error("{}: An exception occurred while loading {}", request.getRemoteAddr(), Util.getAnonymizedURLForRequest(request), e); return null; } diff --git a/airsonic-main/src/main/java/org/airsonic/player/util/Util.java b/airsonic-main/src/main/java/org/airsonic/player/util/Util.java index 0a758f03..7fdb8f37 100644 --- a/airsonic-main/src/main/java/org/airsonic/player/util/Util.java +++ b/airsonic-main/src/main/java/org/airsonic/player/util/Util.java @@ -23,6 +23,8 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.util.MultiValueMap; +import org.springframework.web.util.UriComponentsBuilder; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; @@ -40,6 +42,7 @@ import java.util.List; public final class Util { private static final Logger LOG = LoggerFactory.getLogger(Util.class); + private static final String URL_SENSITIVE_REPLACEMENT_STRING = ""; /** * Disallow external instantiation. @@ -133,6 +136,27 @@ public final class Util { return url; } + /** + * Return an URL for the given HTTP request, with anonymized sensitive parameters. + * + * @param request An HTTP request instance + * @return The associated anonymized URL + */ + public static String getAnonymizedURLForRequest(HttpServletRequest request) { + + String url = getURLForRequest(request); + UriComponentsBuilder builder = UriComponentsBuilder.fromUriString(url); + MultiValueMap components = builder.build().getQueryParams(); + + // Subsonic REST API authentication (see RESTRequestParameterProcessingFilter) + if (components.containsKey("p")) builder.replaceQueryParam("p", URL_SENSITIVE_REPLACEMENT_STRING); // Cleartext password + if (components.containsKey("t")) builder.replaceQueryParam("t", URL_SENSITIVE_REPLACEMENT_STRING); // Token + if (components.containsKey("s")) builder.replaceQueryParam("s", URL_SENSITIVE_REPLACEMENT_STRING); // Salt + if (components.containsKey("u")) builder.replaceQueryParam("u", URL_SENSITIVE_REPLACEMENT_STRING); // Username + + return builder.build().toUriString(); + } + /** * Return true if the given object is an instance of the class name in argument. * If the class doesn't exist, returns false.