Skip to content
Permalink
Browse files

Add connection property to limit server error detail in exception exc…

…eptions (#1579)

* feat: Add connection property to limit server error detail in exception messages

Adds a boolean connection property, LOG_SERVER_MESSAGE_DETAIL ("logServerMessage") that
controls whether the full error detail, as reported back by the server, is included in
the exception messages. This property can be used to limit potentially sensitive content
in server messages from bubbling up in exception messages and stack traces.

If set to "false" then only the top level error will be included in the exception message.
If set to "true" then all details in the server error will be included.

Default value for the new property is "true" to reflect the prior behavior of logging the
full error detail.

Closes #1577

* fix: Add getter and setter for LOG_SERVER_ERROR_DETAIL to BaseDataSource

* test: Add test for logServerErrorDetail connection property
  • Loading branch information
sehrope authored and davecramer committed Nov 14, 2019
1 parent c99ed12 commit cd0b555c8045fc71e6f4d0fb0f24a2deb726301e
@@ -172,6 +172,12 @@
LOG_UNCLOSED_CONNECTIONS("logUnclosedConnections", "false",
"When connections that are not explicitly closed are garbage collected, log the stacktrace from the opening of the connection to trace the leak source"),

/**
* Whether to include full server error detail in exception messages.
*/
LOG_SERVER_ERROR_DETAIL("logServerErrorDetail", "true",
"Include full server error detail in exception messages. If disabled then only the error itself will be included."),

/**
* Enable optimization that disables column name sanitiser.
*/
@@ -45,6 +45,7 @@
private final PreferQueryMode preferQueryMode;
private AutoSave autoSave;
private boolean flushCacheOnDeallocate = true;
protected final boolean logServerErrorDetail;

// default value for server versions that don't report standard_conforming_strings
private boolean standardConformingStrings = false;
@@ -70,6 +71,7 @@ protected QueryExecutorBase(PGStream pgStream, String user,
String preferMode = PGProperty.PREFER_QUERY_MODE.get(info);
this.preferQueryMode = PreferQueryMode.of(preferMode);
this.autoSave = AutoSave.of(PGProperty.AUTOSAVE.get(info));
this.logServerErrorDetail = PGProperty.LOG_SERVER_ERROR_DETAIL.getBoolean(info);
this.cachedQueryCreateAction = new CachedQueryCreateAction(this);
statementCache = new LruCache<Object, CachedQuery>(
Math.max(0, PGProperty.PREPARED_STATEMENT_CACHE_QUERIES.getInt(info)),
@@ -517,7 +517,7 @@ private void doAuthentication(PGStream pgStream, String host, String user, Prope
ServerErrorMessage errorMsg =
new ServerErrorMessage(pgStream.receiveErrorString(elen - 4));
LOGGER.log(Level.FINEST, " <=BE ErrorMessage({0})", errorMsg);
throw new PSQLException(errorMsg);
throw new PSQLException(errorMsg, PGProperty.LOG_SERVER_ERROR_DETAIL.getBoolean(info));

case 'R':
// Authentication request.
@@ -647,7 +647,8 @@ private void doAuthentication(PGStream pgStream, String host, String user, Prope
org.postgresql.gss.MakeGSS.authenticate(pgStream, host, user, password,
PGProperty.JAAS_APPLICATION_NAME.get(info),
PGProperty.KERBEROS_SERVER_NAME.get(info), usespnego,
PGProperty.JAAS_LOGIN.getBoolean(info));
PGProperty.JAAS_LOGIN.getBoolean(info),
PGProperty.LOG_SERVER_ERROR_DETAIL.getBoolean(info));
}
break;

@@ -2500,7 +2500,7 @@ private SQLException receiveErrorResponse() throws IOException {
LOGGER.log(Level.FINEST, " <=BE ErrorMessage({0})", errorMsg.toString());
}

PSQLException error = new PSQLException(errorMsg);
PSQLException error = new PSQLException(errorMsg, this.logServerErrorDetail);
if (transactionFailCause == null) {
transactionFailCause = error;
} else {
@@ -928,6 +928,22 @@ public void setLogUnclosedConnections(boolean enabled) {
PGProperty.LOG_UNCLOSED_CONNECTIONS.set(properties, enabled);
}

/**
* @return true if driver should log include detail in server error messages
* @see PGProperty#LOG_SERVER_ERROR_DETAIL
*/
public boolean getLogServerErrorDetail() {
return PGProperty.LOG_SERVER_ERROR_DETAIL.getBoolean(properties);
}

/**
* @param enabled true if driver should include detail in server error messages
* @see PGProperty#LOG_SERVER_ERROR_DETAIL
*/
public void setLogServerErrorDetail(boolean enabled) {
PGProperty.LOG_SERVER_ERROR_DETAIL.set(properties, enabled);
}

/**
* @return assumed minimal server version
* @see PGProperty#ASSUME_MIN_SERVER_VERSION
@@ -32,15 +32,17 @@
private final String kerberosServerName;
private final boolean useSpnego;
private final GSSCredential clientCredentials;
private final boolean logServerErrorDetail;

GssAction(PGStream pgStream, GSSCredential clientCredentials, String host, String user,
String kerberosServerName, boolean useSpnego) {
String kerberosServerName, boolean useSpnego, boolean logServerErrorDetail) {
this.pgStream = pgStream;
this.clientCredentials = clientCredentials;
this.host = host;
this.user = user;
this.kerberosServerName = kerberosServerName;
this.useSpnego = useSpnego;
this.logServerErrorDetail = logServerErrorDetail;
}

private static boolean hasSpnegoSupport(GSSManager manager) throws GSSException {
@@ -111,7 +113,7 @@ public Exception run() {

LOGGER.log(Level.FINEST, " <=BE ErrorMessage({0})", errorMsg);

return new PSQLException(errorMsg);
return new PSQLException(errorMsg, logServerErrorDetail);
case 'R':
LOGGER.log(Level.FINEST, " <=BE AuthenticationGSSContinue");
int len = pgStream.receiveInteger4();
@@ -28,7 +28,8 @@
private static final Logger LOGGER = Logger.getLogger(MakeGSS.class.getName());

public static void authenticate(PGStream pgStream, String host, String user, String password,
String jaasApplicationName, String kerberosServerName, boolean useSpnego, boolean jaasLogin)
String jaasApplicationName, String kerberosServerName, boolean useSpnego, boolean jaasLogin,
boolean logServerErrorDetail)
throws IOException, SQLException {
LOGGER.log(Level.FINEST, " <=BE AuthenticationReqGSS");

@@ -58,7 +59,7 @@ public static void authenticate(PGStream pgStream, String host, String user, Str
sub = lc.getSubject();
}
PrivilegedAction<Exception> action = new GssAction(pgStream, gssCredential, host, user,
kerberosServerName, useSpnego);
kerberosServerName, useSpnego, logServerErrorDetail);

result = Subject.doAs(sub, action);
} catch (Exception e) {
@@ -20,7 +20,11 @@ public PSQLException(String msg, PSQLState state) {
}

public PSQLException(ServerErrorMessage serverError) {
super(serverError.toString(), serverError.getSQLState());
this(serverError, true);
}

public PSQLException(ServerErrorMessage serverError, boolean detail) {
super(detail ? serverError.toString() : serverError.getNonSensitiveErrorMessage(), serverError.getSQLState());
this.serverError = serverError;
}

@@ -143,6 +143,19 @@ private int getIntegerPart(Character c) {
return Integer.parseInt(s);
}

String getNonSensitiveErrorMessage() {
StringBuilder totalMessage = new StringBuilder();
String message = mesgParts.get(SEVERITY);
if (message != null) {
totalMessage.append(message).append(": ");
}
message = mesgParts.get(MESSAGE);
if (message != null) {
totalMessage.append(message);
}
return totalMessage.toString();
}

public String toString() {
// Now construct the message from what the server sent
// The general format is:
@@ -0,0 +1,100 @@
/*
* Copyright (c) 2019, PostgreSQL Global Development Group
* See the LICENSE file in the project root for more information.
*/

package org.postgresql.test.core;

import org.postgresql.PGProperty;
import org.postgresql.core.ServerVersion;
import org.postgresql.test.TestUtil;

import org.junit.Assert;
import org.junit.Assume;
import org.junit.Test;

import java.sql.Connection;
import java.sql.SQLException;
import java.util.Properties;

public class LogServerMessagePropertyTest {
private static final String PRIMARY_KEY_NAME = "lms_test_pk";
private static final String CREATE_TABLE_SQL =
"CREATE TABLE pg_temp.lms_test ("
+ " id text, "
+ " CONSTRAINT " + PRIMARY_KEY_NAME + " PRIMARY KEY (id)"
+ ")";
private static final String SECRET_VALUE = "some_secret_value";
private static final String INSERT_SQL =
"INSERT INTO pg_temp.lms_test (id) VALUES ('" + SECRET_VALUE + "')";
private static final String UNIQUE_VIOLATION_SQL_STATE = "23505";

/**
* Creates a connection with the additional properties, use it to
* create a temp table with a primary key, run two inserts to generate
* a duplicate key error, and finally return the exception message.
*/
private static String testViolatePrimaryKey(Properties props) throws SQLException {
Connection conn = TestUtil.openDB(props);
Assume.assumeTrue(TestUtil.haveMinimumServerVersion(conn, ServerVersion.v9_1));
try {
TestUtil.execute(CREATE_TABLE_SQL, conn);
// First insert should work
TestUtil.execute(INSERT_SQL, conn);
// Second insert should throw a duplicate key error
TestUtil.execute(INSERT_SQL, conn);
} catch (SQLException e) {
Assert.assertEquals("SQL state must be for a unique violation", UNIQUE_VIOLATION_SQL_STATE, e.getSQLState());
return e.getMessage();
} finally {
conn.close();
}
// Should never get here:
Assert.fail("A duplicate key exception should have occurred");
return null;
}

private static void assertMessageContains(String message, String text) {
if (message.toLowerCase().indexOf(text.toLowerCase()) < 0) {
Assert.fail(String.format("Message must contain text '%s': %s", text, message));
}
}

private static void assertMessageDoesNotContain(String message, String text) {
if (message.toLowerCase().indexOf(text.toLowerCase()) >= 0) {
Assert.fail(String.format("Message must not contain text '%s': %s", text, message));
}
}

@Test
public void testWithDefaults() throws SQLException {
Properties props = new Properties();
String message = testViolatePrimaryKey(props);
assertMessageContains(message, PRIMARY_KEY_NAME);
assertMessageContains(message, "Detail:");
assertMessageContains(message, SECRET_VALUE);
}

/**
* NOTE: This should be the same as the default case as "true" is the default.
*/
@Test
public void testWithExplicitlyEnabled() throws SQLException {
Properties props = new Properties();
props.setProperty(PGProperty.LOG_SERVER_ERROR_DETAIL.getName(), "true");
String message = testViolatePrimaryKey(props);
assertMessageContains(message, PRIMARY_KEY_NAME);
assertMessageContains(message, "Detail:");
assertMessageContains(message, SECRET_VALUE);
}

@Test
public void testWithLogServerErrorDetailDisabled() throws SQLException {
Properties props = new Properties();
props.setProperty(PGProperty.LOG_SERVER_ERROR_DETAIL.getName(), "false");
String message = testViolatePrimaryKey(props);
assertMessageContains(message, PRIMARY_KEY_NAME);
assertMessageDoesNotContain(message, "Detail:");
assertMessageDoesNotContain(message, SECRET_VALUE);
}
}
@@ -15,6 +15,7 @@
import org.postgresql.jdbc.DeepBatchedInsertStatementTest;
import org.postgresql.jdbc.PrimitiveArraySupportTest;
import org.postgresql.test.core.JavaVersionTest;
import org.postgresql.test.core.LogServerMessagePropertyTest;
import org.postgresql.test.core.NativeQueryBindLengthTest;
import org.postgresql.test.core.OptionsPropertyTest;
import org.postgresql.test.util.ExpressionPropertiesTest;
@@ -67,6 +68,7 @@
JavaVersionTest.class,
JBuilderTest.class,
LoginTimeoutTest.class,
LogServerMessagePropertyTest.class,
LruCacheTest.class,
MiscTest.class,
NativeQueryBindLengthTest.class,

0 comments on commit cd0b555

Please sign in to comment.
You can’t perform that action at this time.