Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Switch defaults and model for logging sensitive data

Browse Source 
Issue: SPR-17029
This commit is contained in:
Rossen Stoyanchev
2018-07-10 16:18:31 -04:00
parent a40d25a760
commit 1b1bc7f5b5
34 changed files with 241 additions and 229 deletions
Download Patch File Download Diff File Expand all files Collapse all files

62
spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java
View File

@@ -309,9 +309,6 @@ public class DispatcherServlet extends FrameworkServlet {
/** Perform cleanup of request attributes after include request?. */
private boolean cleanupAfterInclude = true;
/** Do not log potentially sensitive information (params at DEBUG and headers at TRACE). */
private boolean disableLoggingRequestDetails = false;
/** MultipartResolver used by this servlet. */
@Nullable
private MultipartResolver multipartResolver;
@@ -487,25 +484,6 @@ public class DispatcherServlet extends FrameworkServlet {
this.cleanupAfterInclude = cleanupAfterInclude;
}
/**
* Set whether the {@code DispatcherServlet} should not log request
* parameters and headers. By default request parameters are logged at DEBUG
* while headers are logged at TRACE under the log category
* {@code "org.springframework.web.servlet.DispatcherServlet"}. Those may
* contain sensitive information, however this is typically not a problem
* since DEBUG and TRACE are only expected to be enabled in development.
* This property may be used to explicitly disable logging of such
* information regardless of the log level.
* <p>By default this is set to {@code false} in which case request details
* are logged. If set to {@code true} request details will not be logged at
* any log level.
* @param disableLoggingRequestDetails whether to disable or not
* @since 5.1
*/
public void setDisableLoggingRequestDetails(boolean disableLoggingRequestDetails) {
this.disableLoggingRequestDetails = disableLoggingRequestDetails;
}
/**
* This implementation calls {@link #initStrategies}.
@@ -529,20 +507,6 @@ public class DispatcherServlet extends FrameworkServlet {
initRequestToViewNameTranslator(context);
initViewResolvers(context);
initFlashMapManager(context);
if (logger.isDebugEnabled()) {
if (this.disableLoggingRequestDetails) {
logger.debug("Logging request parameters and headers is OFF.");
}
else {
logger.warn("\n\n" +
"!!!!!!!!!!!!!!!!!!!\n" +
"Logging request parameters (DEBUG) and headers (TRACE) may show sensitive data.\n" +
"If not in development, use the DispatcherServlet property \"disableLoggingRequestDetails=true\",\n" +
"or lower the log level.\n" +
"!!!!!!!!!!!!!!!!!!!\n");
}
}
}
/**
@@ -990,26 +954,32 @@ public class DispatcherServlet extends FrameworkServlet {
private void logRequest(HttpServletRequest request) {
if (logger.isDebugEnabled()) {
String params = "";
if (!this.disableLoggingRequestDetails) {
String params;
if (isEnableLoggingRequestDetails()) {
params = request.getParameterMap().entrySet().stream()
.map(entry -> entry.getKey() + ":" + Arrays.toString(entry.getValue()))
.collect(Collectors.joining(", ", ", parameters={", "}"));
.collect(Collectors.joining(", "));
}
else {
params = request.getParameterMap().isEmpty() ? "" : "masked";
}
String query = StringUtils.isEmpty(request.getQueryString()) ? "" : "?" + request.getQueryString();
String dispatchType = !request.getDispatcherType().equals(DispatcherType.REQUEST) ?
"\"" + request.getDispatcherType().name() + "\" dispatch for " : "";
String message = dispatchType + request.getMethod() + " \"" + getRequestUri(request) + "\"" + params;
String message = dispatchType + request.getMethod() +
" \"" + getRequestUri(request) + query + "\", parameters={" + params + "}";
if (logger.isTraceEnabled()) {
String headers = "";
if (!this.disableLoggingRequestDetails) {
headers = Collections.list(request.getHeaderNames()).stream()
.map(name -> name + ":" + Collections.list(request.getHeaders(name)))
.collect(Collectors.joining(", ", ", headers={", "}"));
List<String> values = Collections.list(request.getHeaderNames());
String headers = values.size() > 0 ? "masked" : "";
if (isEnableLoggingRequestDetails()) {
headers = values.stream().map(name -> name + ":" + Collections.list(request.getHeaders(name)))
.collect(Collectors.joining(", "));
}
logger.trace(message + headers + " in DispatcherServlet '" + getServletName() + "'");
logger.trace(message + ", headers={" + headers + "} in DispatcherServlet '" + getServletName() + "'");
}
else {
logger.debug(message);

119
spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java
View File

@@ -19,8 +19,10 @@ package org.springframework.web.servlet;
import java.io.IOException;
import java.security.Principal;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.stream.Collectors;
import javax.servlet.DispatcherType;
import javax.servlet.ServletContext;
import javax.servlet.ServletException;
@@ -217,6 +219,9 @@ public abstract class FrameworkServlet extends HttpServletBean implements Applic
/** Flag used to detect whether onRefresh has already been called. */
private boolean refreshEventReceived = false;
/** Whether to log potentially sensitive info (request params at DEBUG + headers at TRACE). */
private boolean enableLoggingRequestDetails = false;
/**
* Create a new {@code FrameworkServlet} that will create its own internal web
@@ -467,6 +472,26 @@ public abstract class FrameworkServlet extends HttpServletBean implements Applic
this.dispatchTraceRequest = dispatchTraceRequest;
}
/**
* Whether to log request params at DEBUG level, and headers at TRACE level.
* Both may contain sensitive information.
* <p>By default set to {@code false} so that request details are not shown.
* @param enable whether to enable or not
* @since 5.1
*/
public void setEnableLoggingRequestDetails(boolean enable) {
this.enableLoggingRequestDetails = enable;
}
/**
* Whether logging of potentially sensitive, request details at DEBUG and
* TRACE level is allowed.
* @since 5.1
*/
public boolean isEnableLoggingRequestDetails() {
return this.enableLoggingRequestDetails;
}
/**
* Called by Spring via {@link ApplicationContextAware} to inject the current
* application context. This method allows FrameworkServlets to be registered as
@@ -506,6 +531,14 @@ public abstract class FrameworkServlet extends HttpServletBean implements Applic
throw ex;
}
if (logger.isDebugEnabled()) {
String value = this.enableLoggingRequestDetails ?
"shown which may lead to unsafe logging of potentially sensitive data" :
"masked to prevent unsafe logging of potentially sensitive data";
logger.debug("enableLoggingRequestDetails='" + this.enableLoggingRequestDetails +
"': request parameters and headers will be " + value);
}
if (logger.isInfoEnabled()) {
long elapsedTime = System.currentTimeMillis() - startTime;
logger.info("FrameworkServlet '" + getServletName() + "': initialization completed in " +
@@ -989,38 +1022,7 @@ public abstract class FrameworkServlet extends HttpServletBean implements Applic
if (requestAttributes != null) {
requestAttributes.requestCompleted();
}
if (logger.isDebugEnabled()) {
boolean isRequestDispatch = request.getDispatcherType().equals(DispatcherType.REQUEST);
String dispatchType = request.getDispatcherType().name();
if (failureCause != null) {
if (!isRequestDispatch) {
logger.debug("Unresolved failure from \"" + dispatchType + "\" dispatch: " + failureCause);
}
else if (logger.isTraceEnabled()) {
logger.trace("Failed to complete request", failureCause);
}
else {
logger.debug("Failed to complete request: " + failureCause);
}
}
else {
if (asyncManager.isConcurrentHandlingStarted()) {
logger.debug("Exiting but response remains open for further handling");
}
else {
int status = response.getStatus();
if (!isRequestDispatch) {
logger.debug("Exiting from \"" + dispatchType + "\" dispatch (status " + status + ")");
}
else {
HttpStatus httpStatus = HttpStatus.resolve(status);
logger.debug("Completed " + (httpStatus != null ? httpStatus : status));
}
}
}
}
logResult(request, response, failureCause, asyncManager);
publishRequestHandledEvent(request, response, startTime, failureCause);
}
}
@@ -1084,6 +1086,59 @@ public abstract class FrameworkServlet extends HttpServletBean implements Applic
}
}
private void logResult(HttpServletRequest request, HttpServletResponse response,
@Nullable Throwable failureCause, WebAsyncManager asyncManager) {
if (!logger.isDebugEnabled()) {
return;
}
String dispatchType = request.getDispatcherType().name();
boolean initialDispatch = request.getDispatcherType().equals(DispatcherType.REQUEST);
if (failureCause != null) {
if (!initialDispatch) {
// FORWARD/ERROR/ASYNC: minimal message (there should be enough context already)
logger.debug("Unresolved failure from \"" + dispatchType + "\" dispatch: " + failureCause);
}
else if (logger.isTraceEnabled()) {
logger.trace("Failed to complete request", failureCause);
}
else {
logger.debug("Failed to complete request: " + failureCause);
}
return;
}
if (asyncManager.isConcurrentHandlingStarted()) {
logger.debug("Exiting but response remains open for further handling");
return;
}
int status = response.getStatus();
String headers = ""; // nothing below trace
if (logger.isTraceEnabled()) {
Collection<String> names = response.getHeaderNames();
if (this.enableLoggingRequestDetails) {
headers = names.stream().map(name -> name + ":" + response.getHeaders(name))
.collect(Collectors.joining(", "));
}
else {
headers = names.isEmpty() ? "" : "masked";
}
headers = ", headers={" + headers + "}";
}
if (!initialDispatch) {
logger.debug("Exiting from \"" + dispatchType + "\" dispatch, status " + status + headers);
}
else {
HttpStatus httpStatus = HttpStatus.resolve(status);
logger.debug("Completed " + (httpStatus != null ? httpStatus : status) + headers);
}
}
private void publishRequestHandledEvent(HttpServletRequest request, HttpServletResponse response,
long startTime, @Nullable Throwable failureCause) {