Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Revert "Replace the logging logic with SLF4J #30 (#33)"
Browse files Browse the repository at this point in the history
This reverts commit b5fa6c8.
forgedhallpass authored Oct 19, 2021

Verified

This commit was signed with the committer’s verified signature.
mcollina Matteo Collina
1 parent b5fa6c8 commit e2f83ce
Showing 21 changed files with 392 additions and 98 deletions.
Original file line number Diff line number Diff line change
@@ -28,9 +28,6 @@
*/
package org.owasp.csrfguard.test;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@@ -54,8 +51,6 @@ public class CounterServlet extends HttpServlet {
public static final String INPUT_PARAMETER_NAME = "value";
public static final String NO_SESSION_MESSAGE = "The request did not contain an associated session!";

private static final Logger LOGGER = LoggerFactory.getLogger(CounterServlet.class);

/**
* Default constructor.
*/
@@ -70,7 +65,7 @@ protected void doGet(final HttpServletRequest request, final HttpServletResponse
if (Objects.nonNull(session)) {
respond(response, COUNTER.compute(session.getId(), (k, v) -> Optional.ofNullable(v).orElse(0)));
} else {
LOGGER.error(NO_SESSION_MESSAGE);
System.err.println(NO_SESSION_MESSAGE);
response.setStatus(HttpServletResponse.SC_FORBIDDEN);
}
}
@@ -89,7 +84,7 @@ protected void doPost(final HttpServletRequest request, final HttpServletRespons
respond(response, COUNTER.compute(session.getId(), (k, v) -> Objects.isNull(v) ? valueToIncreaseBy : v + valueToIncreaseBy));
} catch (Exception ignored) {}
} else {
LOGGER.error(NO_SESSION_MESSAGE);
System.err.println(NO_SESSION_MESSAGE);
response.setStatus(HttpServletResponse.SC_FORBIDDEN);
}
}
Original file line number Diff line number Diff line change
@@ -35,6 +35,21 @@
# if there isn't a context it will be the empty string. So to use this in the configuration, use e.g. %servletContext%/something.html
# which will translate to e.g. /someApp/something.html

############
## Logger ##
############
# The logger property (org.owasp.csrfguard.Logger) defines the qualified class name of
# the object responsible for processing all log messages produced by CSRFGuard. The default
# CSRFGuard logger is org.owasp.csrfguard.log.ConsoleLogger. This class logs all messages
# to System.out which JavaEE application servers redirect to a vendor specific log file.
# Developers can customize the logging behavior of CSRFGuard by implementing the
# org.owasp.csrfguard.log.ILogger interface and setting the logger property to the new
# logger's qualified class name. The following configuration snippet instructs OWASP CSRFGuard
# to capture all log messages to the console:
#
# org.owasp.csrfguard.Logger = org.owasp.csrfguard.log.ConsoleLogger
org.owasp.csrfguard.Logger = org.owasp.csrfguard.log.JavaLogger

# Which configuration provider factory you want to use. The default is org.owasp.csrfguard.config.PropertiesConfigurationProviderFactory
# Another configuration provider has more features including config overlays: org.owasp.csrfguard.config.overlay.ConfigurationOverlayProviderFactory
# The default configuration provider is: org.owasp.csrfguard.config.overlay.ConfigurationAutodetectProviderFactory
4 changes: 0 additions & 4 deletions csrfguard/pom.xml
Original file line number Diff line number Diff line change
@@ -76,10 +76,6 @@
<groupId>org.mockito</groupId>
<artifactId>mockito-inline</artifactId>
</dependency>
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
</dependency>
</dependencies>

<build>
5 changes: 5 additions & 0 deletions csrfguard/src/main/java/org/owasp/csrfguard/CsrfGuard.java
Original file line number Diff line number Diff line change
@@ -36,6 +36,7 @@
import org.owasp.csrfguard.config.PropertiesConfigurationProviderFactory;
import org.owasp.csrfguard.config.overlay.ExpirableCache;
import org.owasp.csrfguard.config.properties.ConfigParameters;
import org.owasp.csrfguard.log.ILogger;
import org.owasp.csrfguard.session.LogicalSession;
import org.owasp.csrfguard.token.service.TokenService;
import org.owasp.csrfguard.token.storage.LogicalSessionExtractor;
@@ -80,6 +81,10 @@ public Map<String, Pattern> getRegexPatternCache() {
return this.regexPatternCache;
}

public ILogger getLogger() {
return config().getLogger();
}

public String getTokenName() {
return config().getTokenName();
}
19 changes: 8 additions & 11 deletions csrfguard/src/main/java/org/owasp/csrfguard/CsrfGuardFilter.java
Original file line number Diff line number Diff line change
@@ -30,12 +30,11 @@
package org.owasp.csrfguard;

import org.owasp.csrfguard.http.InterceptRedirectResponse;
import org.owasp.csrfguard.log.LogLevel;
import org.owasp.csrfguard.session.LogicalSession;
import org.owasp.csrfguard.token.storage.LogicalSessionExtractor;
import org.owasp.csrfguard.token.transferobject.TokenTO;
import org.owasp.csrfguard.util.CsrfGuardUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.servlet.*;
import javax.servlet.http.HttpServletRequest;
@@ -48,8 +47,6 @@ public class CsrfGuardFilter implements Filter {

private FilterConfig filterConfig = null;

private static final Logger LOGGER = LoggerFactory.getLogger(CsrfGuardFilter.class);

@Override
public void init(final FilterConfig filterConfig) {
this.filterConfig = filterConfig;
@@ -63,7 +60,7 @@ public void doFilter(final ServletRequest request, final ServletResponse respons
if (request instanceof HttpServletRequest && response instanceof HttpServletResponse) {
doFilter((HttpServletRequest) request, (HttpServletResponse) response, filterChain, csrfGuard);
} else {
handleNonHttpServletMessages(request, response, filterChain);
handleNonHttpServletMessages(request, response, filterChain, csrfGuard);
}
} else {
filterChain.doFilter(request, response);
@@ -98,7 +95,7 @@ private void handleSession(final HttpServletRequest httpServletRequest, final In
} else if (new CsrfValidator().isValid(httpServletRequest, interceptRedirectResponse)) {
filterChain.doFilter(httpServletRequest, interceptRedirectResponse);
} else {
logInvalidRequest(httpServletRequest);
logInvalidRequest(httpServletRequest, csrfGuard);
}

// TODO this is not needed in case of un-protected pages
@@ -114,25 +111,25 @@ private void handleNoSession(final HttpServletRequest httpServletRequest, final
if (new CsrfValidator().isValid(httpServletRequest, interceptRedirectResponse)) {
filterChain.doFilter(httpServletRequest, interceptRedirectResponse);
} else {
logInvalidRequest(httpServletRequest);
logInvalidRequest(httpServletRequest, csrfGuard);
}
} else {
filterChain.doFilter(httpServletRequest, httpServletResponse);
}
}

private void handleNonHttpServletMessages(final ServletRequest request, final ServletResponse response, final FilterChain filterChain) throws IOException, ServletException {
private void handleNonHttpServletMessages(final ServletRequest request, final ServletResponse response, final FilterChain filterChain, final CsrfGuard csrfGuard) throws IOException, ServletException {
final String message = String.format("CSRFGuard does not know how to work with requests of class %s ", request.getClass().getName());
LOGGER.warn(message);
csrfGuard.getLogger().log(LogLevel.Warning, message);
this.filterConfig.getServletContext().log("[WARNING]" + message);

filterChain.doFilter(request, response);
}

private void logInvalidRequest(final HttpServletRequest httpRequest) {
private void logInvalidRequest(final HttpServletRequest httpRequest, final CsrfGuard csrfGuard) {
final String requestURI = httpRequest.getRequestURI();
final String remoteAddress = httpRequest.getRemoteAddr();

LOGGER.warn(String.format("Invalid request: URI: '%s' | Remote Address: '%s'", requestURI, remoteAddress));
csrfGuard.getLogger().log(LogLevel.Warning, String.format("Invalid request: URI: '%s' | Remote Address: '%s'", requestURI, remoteAddress));
}
}
13 changes: 6 additions & 7 deletions csrfguard/src/main/java/org/owasp/csrfguard/CsrfValidator.java
Original file line number Diff line number Diff line change
@@ -30,6 +30,8 @@

import org.apache.commons.lang3.StringUtils;
import org.owasp.csrfguard.action.IAction;
import org.owasp.csrfguard.log.ILogger;
import org.owasp.csrfguard.log.LogLevel;
import org.owasp.csrfguard.servlet.JavaScriptServlet;
import org.owasp.csrfguard.session.LogicalSession;
import org.owasp.csrfguard.token.businessobject.TokenBO;
@@ -39,8 +41,6 @@
import org.owasp.csrfguard.util.CsrfGuardUtils;
import org.owasp.csrfguard.util.MessageConstants;
import org.owasp.csrfguard.util.RegexValidationUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@@ -53,22 +53,21 @@ public final class CsrfValidator {

private final CsrfGuard csrfGuard;

private static final Logger LOGGER = LoggerFactory.getLogger(CsrfValidator.class);

public CsrfValidator() {
this.csrfGuard = CsrfGuard.getInstance();
}

public boolean isValid(final HttpServletRequest request, final HttpServletResponse response) {
final boolean isValid;

final ILogger logger = this.csrfGuard.getLogger();
final String normalizedResourceURI = CsrfGuardUtils.normalizeResourceURI(request);
final ProtectionResult protectionResult = isProtectedPageAndMethod(request);
if (protectionResult.isProtected()) {
LOGGER.debug(String.format("CSRFGuard analyzing protected resource: '%s'", normalizedResourceURI));
logger.log(LogLevel.Debug, String.format("CSRFGuard analyzing protected resource: '%s'", normalizedResourceURI));
isValid = isTokenValidInRequest(request, response, protectionResult.getResourceIdentifier());
} else {
LOGGER.debug(String.format("Unprotected page: %s", normalizedResourceURI));
logger.log(LogLevel.Debug, String.format("Unprotected page: %s", normalizedResourceURI));
isValid = true;
}

@@ -241,7 +240,7 @@ private void callActionsOnError(final HttpServletRequest request, final HttpServ
try {
action.execute(request, response, csrfGuardException, this.csrfGuard);
} catch (final CsrfGuardException exception) {
LOGGER.error(String.valueOf(exception));
this.csrfGuard.getLogger().log(LogLevel.Error, exception);
}
}
}
7 changes: 2 additions & 5 deletions csrfguard/src/main/java/org/owasp/csrfguard/action/Log.java
Original file line number Diff line number Diff line change
@@ -32,8 +32,7 @@
import org.apache.commons.lang3.StringUtils;
import org.owasp.csrfguard.CsrfGuard;
import org.owasp.csrfguard.CsrfGuardException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.owasp.csrfguard.log.LogLevel;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@@ -43,8 +42,6 @@ public final class Log extends AbstractAction {

private static final long serialVersionUID = 8238761463376338707L;

private static final Logger LOGGER = LoggerFactory.getLogger(Log.class);

@Override
public void execute(final HttpServletRequest request, final HttpServletResponse response, final CsrfGuardException csrfe, final CsrfGuard csrfGuard) throws CsrfGuardException {
String logMessage = getParameter("Message");
@@ -70,7 +67,7 @@ public void execute(final HttpServletRequest request, final HttpServletResponse

logMessage = logMessage.replace("%user%", getUserName(request));

LOGGER.error(logMessage);
csrfGuard.getLogger().log(LogLevel.Error, logMessage);
}

private String getUserName(final HttpServletRequest request) {
Original file line number Diff line number Diff line change
@@ -30,6 +30,7 @@
package org.owasp.csrfguard.config;

import org.owasp.csrfguard.action.IAction;
import org.owasp.csrfguard.log.ILogger;
import org.owasp.csrfguard.token.storage.LogicalSessionExtractor;
import org.owasp.csrfguard.token.storage.TokenHolder;

@@ -58,6 +59,13 @@ public interface ConfigurationProvider {
*/
boolean isPrintConfig();

/**
* TODO document
*
* @return
*/
ILogger getLogger();

/**
* TODO document
*
Original file line number Diff line number Diff line change
@@ -31,6 +31,8 @@

import org.owasp.csrfguard.action.IAction;
import org.owasp.csrfguard.config.properties.ConfigParameters;
import org.owasp.csrfguard.log.ConsoleLogger;
import org.owasp.csrfguard.log.ILogger;
import org.owasp.csrfguard.token.storage.LogicalSessionExtractor;
import org.owasp.csrfguard.token.storage.TokenHolder;

@@ -47,6 +49,8 @@
*/
public final class NullConfigurationProvider implements ConfigurationProvider {

private static final ILogger LOGGER = new ConsoleLogger();

public NullConfigurationProvider() {}

@Override
@@ -59,6 +63,11 @@ public boolean isPrintConfig() {
return false;
}

@Override
public ILogger getLogger() {
return LOGGER;
}

@Override
public String getTokenName() {
return null;
Original file line number Diff line number Diff line change
@@ -37,13 +37,13 @@
import org.owasp.csrfguard.config.properties.PropertyUtils;
import org.owasp.csrfguard.config.properties.javascript.JavaScriptConfigParameters;
import org.owasp.csrfguard.config.properties.javascript.JsConfigParameter;
import org.owasp.csrfguard.log.ILogger;
import org.owasp.csrfguard.log.LogLevel;
import org.owasp.csrfguard.servlet.JavaScriptServlet;
import org.owasp.csrfguard.token.storage.LogicalSessionExtractor;
import org.owasp.csrfguard.token.storage.TokenHolder;
import org.owasp.csrfguard.util.CsrfGuardUtils;
import org.owasp.csrfguard.util.RegexValidationUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.servlet.ServletConfig;
import java.io.IOException;
@@ -60,7 +60,7 @@
*/
public class PropertiesConfigurationProvider implements ConfigurationProvider {

private static final Logger LOGGER = LoggerFactory.getLogger(PropertiesConfigurationProvider.class);
private final ILogger logger;

private final Set<String> protectedPages;

@@ -151,6 +151,8 @@ public PropertiesConfigurationProvider(final Properties properties) {
this.protectedMethods = new HashSet<>();
this.unprotectedMethods = new HashSet<>();

this.logger = CsrfGuardUtils.<ILogger>forName(PropertyUtils.getProperty(properties, ConfigParameters.LOGGER)).newInstance();

this.enabled = PropertyUtils.getProperty(properties, ConfigParameters.CSRFGUARD_ENABLED);

if (this.enabled) {
@@ -190,6 +192,11 @@ public PropertiesConfigurationProvider(final Properties properties) {
}
}

@Override
public ILogger getLogger() {
return this.logger;
}

@Override
public String getTokenName() {
return this.tokenName;
@@ -530,7 +537,7 @@ private String getPageProperty(final Properties properties, final String propert
private boolean isSpecialUriDescriptor(final String resourceUri) {
if (this.tokenPerPage && (resourceUri.endsWith("/*") || resourceUri.startsWith("*."))) {
// FIXME implement in the JS logic (calculatePageTokenForUri)
LOGGER.warn("'Extension' and 'partial path wildcard' matching for page tokens is not supported properly yet! " +
this.logger.log(LogLevel.Warning, "'Extension' and 'partial path wildcard' matching for page tokens is not supported properly yet! " +
"Every resource will be assigned a new unique token instead of using the defined resource matcher token. " +
"Although this is not a security issue, in case of a large REST application it can have an impact on performance. " +
"Consider using regular expressions instead.");
@@ -623,14 +630,14 @@ private SecureRandom getSecureRandomInstance(final String algorithm, final Strin
secureRandom = Objects.nonNull(algorithm) ? SecureRandom.getInstance(algorithm) : getDefaultSecureRandom();
}
} catch (final NoSuchProviderException e) {
LOGGER.warn(String.format("The configured Secure Random Provider '%s' was not found, trying default providers.", provider));
LOGGER.info(getAvailableSecureRandomProvidersAndAlgorithms());
this.logger.log(LogLevel.Warning, String.format("The configured Secure Random Provider '%s' was not found, trying default providers.", provider));
this.logger.log(LogLevel.Info, getAvailableSecureRandomProvidersAndAlgorithms());

secureRandom = getSecureRandomInstance(algorithm, null);
logDefaultPrngProviderAndAlgorithm(secureRandom);
} catch (final NoSuchAlgorithmException nse) {
LOGGER.warn(String.format("The configured Secure Random Algorithm '%s' was not found, reverting to system defaults.", algorithm));
LOGGER.info(getAvailableSecureRandomProvidersAndAlgorithms());
this.logger.log(LogLevel.Warning, String.format("The configured Secure Random Algorithm '%s' was not found, reverting to system defaults.", algorithm));
this.logger.log(LogLevel.Info, getAvailableSecureRandomProvidersAndAlgorithms());

secureRandom = getSecureRandomInstance(null, null);
}
@@ -644,7 +651,7 @@ private SecureRandom getDefaultSecureRandom() {
}

private void logDefaultPrngProviderAndAlgorithm(final SecureRandom defaultSecureRandom) {
LOGGER.info(String.format("Using default Secure Random Provider '%s' and '%s' Algorithm.", defaultSecureRandom.getProvider().getName(), defaultSecureRandom.getAlgorithm()));
this.logger.log(LogLevel.Info, String.format("Using default Secure Random Provider '%s' and '%s' Algorithm.", defaultSecureRandom.getProvider().getName(), defaultSecureRandom.getAlgorithm()));
}

private static String getAvailableSecureRandomProvidersAndAlgorithms() {
Loading

0 comments on commit e2f83ce

Please sign in to comment.