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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update the logging properties to opt-out of the prefix events #844 #845

Merged
merged 10 commits into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions configuration/esapi/ESAPI.properties
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,10 @@ Logger.UserInfo=true
# Determines whether ESAPI should log the session id and client IP.
Logger.ClientInfo=true

# Determines whether ESAPI should log the prefix of [EVENT_TYPE - APPLICATION NAME].
# If all above Logger entries are set to false, as well as LogPrefix, then the output would be the same as if no ESAPI was used
Logger.LogPrefix=true

#===========================================================================
# ESAPI Intrusion Detection
#
Expand Down
1 change: 1 addition & 0 deletions src/main/java/org/owasp/esapi/PropNames.java
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ public final class PropNames {
public static final String LOG_ENCODING_REQUIRED = "Logger.LogEncodingRequired";
public static final String LOG_APPLICATION_NAME = "Logger.LogApplicationName";
public static final String LOG_SERVER_IP = "Logger.LogServerIP";
public static final String LOG_PREFIX = "Logger.LogPrefix";

public static final String VALIDATION_PROPERTIES = "Validator.ConfigurationFile";
public static final String VALIDATION_PROPERTIES_MULTIVALUED = "Validator.ConfigurationFile.MultiValued";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,24 @@ public class EventTypeLogSupplier // implements Supplier<String>
{
/** EventType reference to supply log representation of. */
private final EventType eventType;
/** Whether to log or not the event type */
private boolean logEventType = true;

/**
* Ctr
*
* @param evtyp EventType reference to supply log representation for
* @param eventType EventType reference to supply log representation for
kwwall marked this conversation as resolved.
Show resolved Hide resolved
*/
public EventTypeLogSupplier(EventType evtyp) {
this.eventType = evtyp == null ? Logger.EVENT_UNSPECIFIED : evtyp;
public EventTypeLogSupplier(EventType eventType) {
this.eventType = eventType == null ? Logger.EVENT_UNSPECIFIED : eventType;
}

// @Override -- Uncomment when we switch to Java 8 as minimal baseline.
public String get() {
return eventType.toString();
return logEventType ? eventType.toString() : "";
}

public void setLogEventType(boolean logEventType) {
this.logEventType = logEventType;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,27 +35,47 @@ public class LogPrefixAppender implements LogAppender {
private final boolean logApplicationName;
/** Application Name to record. */
private final String appName;
/** Whether or not to print the prefix. */
private final boolean logPrefix;

/**
* Ctr.
* Constructor
*
* @param logUserInfo Whether or not to record user information
* @param logClientInfo Whether or not to record client information
* @param logServerIp Whether or not to record server ip information
* @param logApplicationName Whether or not to record application name
* @param appName Application Name to record.
* @param logPrefix is set by default to true
*/
@SuppressWarnings("JavadocReference")
public LogPrefixAppender(boolean logUserInfo, boolean logClientInfo, boolean logServerIp, boolean logApplicationName, String appName) {
this(logUserInfo, logClientInfo, logServerIp, logApplicationName, appName, true);
}

/**
* Constructor
*
* @param logUserInfo Whether or not to record user information
* @param logClientInfo Whether or not to record client information
* @param logServerIp Whether or not to record server ip information
* @param logApplicationName Whether or not to record application name
* @param appName Application Name to record.
* @param logPrefix Whether or not to print the prefix
*/
public LogPrefixAppender(boolean logUserInfo, boolean logClientInfo, boolean logServerIp, boolean logApplicationName, String appName, boolean logPrefix) {
this.logUserInfo = logUserInfo;
this.logClientInfo = logClientInfo;
this.logServerIp = logServerIp;
this.logApplicationName = logApplicationName;
this.appName = appName;
this.logPrefix = logPrefix;
}

@Override
public String appendTo(String logName, EventType eventType, String message) {
EventTypeLogSupplier eventTypeSupplier = new EventTypeLogSupplier(eventType);
eventTypeSupplier.setLogEventType(this.logPrefix);

UserInfoSupplier userInfoSupplier = new UserInfoSupplier();
userInfoSupplier.setLogUserInfo(logUserInfo);
Expand All @@ -66,6 +86,7 @@ public String appendTo(String logName, EventType eventType, String message) {
ServerInfoSupplier serverInfoSupplier = new ServerInfoSupplier(logName);
serverInfoSupplier.setLogServerIp(logServerIp);
serverInfoSupplier.setLogApplicationName(logApplicationName, appName);
serverInfoSupplier.setLogLogName(logPrefix);

String eventTypeMsg = eventTypeSupplier.get().trim();
String userInfoMsg = userInfoSupplier.get().trim();
Expand All @@ -80,17 +101,20 @@ public String appendTo(String logName, EventType eventType, String message) {

String[] optionalPrefixContent = new String[] {userInfoMsg + clientInfoMsg, serverInfoMsg};

StringBuilder logPrefix = new StringBuilder();
//EventType is always appended
logPrefix.append(eventTypeMsg);
StringBuilder logPrefixBuilder = new StringBuilder();
//EventType is always appended (unless we specifically asked not to Log Prefix)
if (this.logPrefix) {
logPrefixBuilder.append(eventTypeMsg);
}

for (String element : optionalPrefixContent) {
if (!element.isEmpty()) {
logPrefix.append(" ");
logPrefix.append(element);
logPrefixBuilder.append(" ");
logPrefixBuilder.append(element);
}
}

return String.format(RESULT_FORMAT, logPrefix.toString(), message);
String logPrefixContent = logPrefixBuilder.toString();
return logPrefixContent.trim().isEmpty() ? message : String.format(RESULT_FORMAT, logPrefixContent, message);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ public class ServerInfoSupplier // implements Supplier<String>
private boolean logAppName = true;
/** The application name to log. */
private String applicationName = "";

/** Whether to log the Name */
private boolean logLogName = true;
/** Reference to the associated logname/module name. */
private final String logName;

Expand All @@ -57,10 +58,14 @@ public String get() {
appInfo.append(request.getLocalAddr()).append(":").append(request.getLocalPort());
}
}
if (logAppName) {
appInfo.append("/").append(applicationName);

if (this.logAppName) {
appInfo.append("/").append(this.applicationName);
}

if (this.logLogName) {
appInfo.append("/").append(logName);
}
appInfo.append("/").append(logName);

return appInfo.toString();
}
Expand All @@ -74,6 +79,15 @@ public void setLogServerIp(boolean log) {
this.logServerIP = log;
}

/**
* Specify whether the instance should record the prefix.
*
* @param logLogName {@code true} to record
*/
public void setLogLogName(boolean logLogName) {
this.logLogName = logLogName;
}

/**
* Specify whether the instance should record the application name
*
Expand Down
27 changes: 26 additions & 1 deletion src/main/java/org/owasp/esapi/logging/java/JavaLogFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static org.owasp.esapi.PropNames.LOG_ENCODING_REQUIRED;
import static org.owasp.esapi.PropNames.LOG_SERVER_IP;
import static org.owasp.esapi.PropNames.LOG_USER_INFO;
import static org.owasp.esapi.PropNames.LOG_PREFIX;

import java.io.IOException;
import java.io.InputStream;
Expand Down Expand Up @@ -79,7 +80,17 @@ public class JavaLogFactory implements LogFactory {
boolean logApplicationName = ESAPI.securityConfiguration().getBooleanProp(LOG_APPLICATION_NAME);
String appName = ESAPI.securityConfiguration().getStringProp(APPLICATION_NAME);
boolean logServerIp = ESAPI.securityConfiguration().getBooleanProp(LOG_SERVER_IP);
JAVA_LOG_APPENDER = createLogAppender(logUserInfo, logClientInfo, logServerIp, logApplicationName, appName);

boolean logPrefix = true;
try {
logPrefix = ESAPI.securityConfiguration().getBooleanProp(LOG_PREFIX);
} catch (ConfigurationException ex) {
System.out.println("ESAPI: Failed to read Log Prefix configuration. Defaulting to enabled" +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change the output to something that explicitly mentions the property name on the output sent to stderr. E.g., something like this:

 System.out.println("ESAPI: Failed to read Log Prefix configuration, " + LOG_PREFIX + ". Defaulting to enabled" +

That will make it easier for those getting this failure to know exactly what property to look for if they wish to change it in their ESAPI.properties file.

". Caught " + ex.getClass().getName() +
"; exception message was: " + ex);
}

JAVA_LOG_APPENDER = createLogAppender(logUserInfo, logClientInfo, logServerIp, logApplicationName, appName, logPrefix);

Map<Integer, JavaLogLevelHandler> levelLookup = new HashMap<>();
levelLookup.put(Logger.ALL, JavaLogLevelHandlers.ALWAYS);
Expand Down Expand Up @@ -144,6 +155,20 @@ public class JavaLogFactory implements LogFactory {
return new LogPrefixAppender(logUserInfo, logClientInfo, logServerIp, logApplicationName, appName);
}

/**
* Populates the default log appender for use in factory-created loggers.
* @param appName
* @param logApplicationName
* @param logServerIp
* @param logClientInfo
* @param logPrefix
*
* @return LogAppender instance.
*/
/*package*/ static LogAppender createLogAppender(boolean logUserInfo, boolean logClientInfo, boolean logServerIp, boolean logApplicationName, String appName, boolean logPrefix) {
return new LogPrefixAppender(logUserInfo, logClientInfo, logServerIp, logApplicationName, appName, logPrefix);
}


@Override
public Logger getLogger(String moduleName) {
Expand Down
27 changes: 26 additions & 1 deletion src/main/java/org/owasp/esapi/logging/slf4j/Slf4JLogFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.owasp.esapi.LogFactory;
import org.owasp.esapi.Logger;
import org.owasp.esapi.codecs.HTMLEntityCodec;
import org.owasp.esapi.errors.ConfigurationException;
import org.owasp.esapi.logging.appender.LogAppender;
import org.owasp.esapi.logging.appender.LogPrefixAppender;
import org.owasp.esapi.logging.cleaning.CodecLogScrubber;
Expand All @@ -36,6 +37,7 @@
import static org.owasp.esapi.PropNames.LOG_APPLICATION_NAME;
import static org.owasp.esapi.PropNames.APPLICATION_NAME;
import static org.owasp.esapi.PropNames.LOG_SERVER_IP;
import static org.owasp.esapi.PropNames.LOG_PREFIX;
import org.slf4j.LoggerFactory;
/**
* LogFactory implementation which creates SLF4J supporting Loggers.
Expand Down Expand Up @@ -69,7 +71,17 @@ public class Slf4JLogFactory implements LogFactory {
boolean logApplicationName = ESAPI.securityConfiguration().getBooleanProp(LOG_APPLICATION_NAME);
String appName = ESAPI.securityConfiguration().getStringProp(APPLICATION_NAME);
boolean logServerIp = ESAPI.securityConfiguration().getBooleanProp(LOG_SERVER_IP);
SLF4J_LOG_APPENDER = createLogAppender(logUserInfo, logClientInfo, logServerIp, logApplicationName, appName);

boolean logPrefix = true;
try {
logPrefix = ESAPI.securityConfiguration().getBooleanProp(LOG_PREFIX);
} catch (ConfigurationException ex) {
System.out.println("ESAPI: Failed to read Log Prefix configuration. Defaulting to enabled" +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as previous comment.

". Caught " + ex.getClass().getName() +
"; exception message was: " + ex);
}

SLF4J_LOG_APPENDER = createLogAppender(logUserInfo, logClientInfo, logServerIp, logApplicationName, appName, logPrefix);

Map<Integer, Slf4JLogLevelHandler> levelLookup = new HashMap<>();
levelLookup.put(Logger.ALL, Slf4JLogLevelHandlers.TRACE);
Expand Down Expand Up @@ -114,6 +126,19 @@ public class Slf4JLogFactory implements LogFactory {
return new LogPrefixAppender(logUserInfo, logClientInfo, logServerIp, logApplicationName, appName);
}

/**
* Populates the default log appender for use in factory-created loggers.
* @param appName
* @param logApplicationName
* @param logServerIp
* @param logClientInfo
* @param logPrefix
*
* @return LogAppender instance.
*/
/*package*/ static LogAppender createLogAppender(boolean logUserInfo, boolean logClientInfo, boolean logServerIp, boolean logApplicationName, String appName, boolean logPrefix) {
return new LogPrefixAppender(logUserInfo, logClientInfo, logServerIp, logApplicationName, appName, logPrefix);
}

@Override
public Logger getLogger(String moduleName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1441,14 +1441,14 @@ public Boolean getBooleanProp(String propertyName) throws ConfigurationException
try {
return esapiPropertyManager.getBooleanProp(propertyName);
} catch (ConfigurationException ex) {
String property = properties.getProperty( propertyName );
String property = properties.getProperty(propertyName);
if ( property == null ) {
throw new ConfigurationException( "SecurityConfiguration for " + propertyName + " not found in ESAPI.properties");
}
if ( property.equalsIgnoreCase("true") || property.equalsIgnoreCase("yes" ) ) {
if ( property.equalsIgnoreCase("true") || property.equalsIgnoreCase("yes") ) {
return true;
}
if ( property.equalsIgnoreCase("false") || property.equalsIgnoreCase( "no" ) ) {
if ( property.equalsIgnoreCase("false") || property.equalsIgnoreCase("no") ) {
return false;
}
throw new ConfigurationException( "SecurityConfiguration for " + propertyName + " has incorrect " +
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package org.owasp.esapi.logging.appender;

import static org.junit.Assert.assertEquals;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.owasp.esapi.Logger;

import java.util.ArrayList;
import java.util.Collection;
import java.util.List;

@RunWith(Parameterized.class)
public class EventTypeLogSupplierIgnoreEventTypeTest {

@Parameterized.Parameters (name="{0} -> {1}")
public static Collection<Object[]> assembleTests() {
List<Object[]> paramSets = new ArrayList<>();
paramSets.add(new Object[] {Logger.EVENT_FAILURE,""});
paramSets.add(new Object[] {Logger.EVENT_SUCCESS,""});
paramSets.add(new Object[] {Logger.EVENT_UNSPECIFIED,""});
paramSets.add(new Object[] {Logger.SECURITY_AUDIT,""});
paramSets.add(new Object[] {Logger.SECURITY_FAILURE,""});
paramSets.add(new Object[] {Logger.SECURITY_SUCCESS,""});
paramSets.add(new Object[] {null, ""});

return paramSets;
}

private final Logger.EventType eventType;
private final String expectedResult;

public EventTypeLogSupplierIgnoreEventTypeTest(Logger.EventType eventType, String result) {
this.eventType = eventType;
this.expectedResult = result;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Add blank line here for improved readability.


@Test
public void testEventTypeLogIgnoreEventType() {
EventTypeLogSupplier supplier = new EventTypeLogSupplier(eventType);
supplier.setLogEventType(false);
assertEquals(expectedResult, supplier.get());
}
}
Loading