This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 878d7dce486ae5b5f5fb0e4fbd8225332cb14ff1 Author: opencode <[email protected]> AuthorDate: Fri Nov 6 09:42:24 2026 +0100 Harden PoolProperties credential handling setPassword(null) removed only the password key, leaving values configured as pass or pwd in the live database properties where they could keep authenticating new connections. Remove every recognized credential alias, including defaults-backed values, while leaving the configured connectionProperties string unchanged. A malformed interceptor definition also exposed its credential-bearing properties in the exception message. The parser now masks credentials in the reported definition. --- .../apache/tomcat/jdbc/pool/PoolProperties.java | 23 +++++++++++-- .../pool/TestJdbcInterceptorConfigParsing.java | 19 ++++++++++ .../tomcat/jdbc/pool/TestPoolProperties.java | 40 ++++++++++++++++++++++ 3 files changed, 79 insertions(+), 3 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolProperties.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolProperties.java index 74cc4a0b41..f934da4416 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolProperties.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolProperties.java @@ -528,7 +528,7 @@ public class PoolProperties implements PoolConfiguration, Cloneable, Serializabl if (pidx < 0) { throw new IllegalArgumentException(MessageFactory.get( "poolProperties.invalidInterceptorProperty", prop.trim(), - interceptorValues[i])); + maskInterceptorCredentials(interceptorValues[i]))); } String propName = prop.substring(0, pidx).trim(); String propValue = prop.substring(pidx + 1).trim(); @@ -762,8 +762,7 @@ public class PoolProperties implements PoolConfiguration, Cloneable, Serializabl this.password = password; if (this.dbProperties != null) { if (password == null) { - removeDbProperty(PoolUtilities.PROP_PASSWORD); - warnStringStillNamesCredential(PoolUtilities.PROP_PASSWORD); + removeCredentialDbProperties(); } else { this.dbProperties.setProperty(PoolUtilities.PROP_PASSWORD, password); } @@ -771,6 +770,24 @@ public class PoolProperties implements PoolConfiguration, Cloneable, Serializabl } + /** + * Removes every database property whose key is recognized as a password alias, so clearing the password also + * stops a value configured as {@code pass} or {@code pwd} from still authenticating connections. + */ + private void removeCredentialDbProperties() { + Properties props = this.dbProperties; + if (props == null) { + return; + } + for (String key : props.stringPropertyNames()) { + if (PoolUtilities.isCredentialKey(key)) { + removeDbProperty(key); + warnStringStillNamesCredential(key); + } + } + } + + @Override public void setRemoveAbandoned(boolean removeAbandoned) { this.removeAbandoned = removeAbandoned; diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestJdbcInterceptorConfigParsing.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestJdbcInterceptorConfigParsing.java index 9ea43d0762..f8a445c879 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestJdbcInterceptorConfigParsing.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestJdbcInterceptorConfigParsing.java @@ -153,6 +153,25 @@ class TestJdbcInterceptorConfigParsing { } } + @Test + void aMalformedInterceptorPropertyMasksTheRestOfTheDefinition() { + String badConfig = "StatementCache(password=secret,bad)"; + PoolProperties props = new PoolProperties(); + props.setJdbcInterceptors(badConfig); + + IllegalArgumentException thrown = Assertions.assertThrows(IllegalArgumentException.class, + props::getJdbcInterceptorsAsArray); + + Assertions.assertFalse(thrown.getMessage().contains("secret"), + "the credential in the same definition must not leak: " + thrown.getMessage()); + Assertions.assertTrue(thrown.getMessage().contains("password=********"), + "the credential must be shown masked: " + thrown.getMessage()); + Assertions.assertTrue(thrown.getMessage().contains("StatementCache"), + "the definition must remain identifiable: " + thrown.getMessage()); + Assertions.assertTrue(thrown.getMessage().contains("bad"), + "the invalid property must remain identifiable: " + thrown.getMessage()); + } + @Test void getPoolReturnsNullForAnUncheckedConfigurationFailure() { DataSourceProxy proxy = new DataSourceProxy(); diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestPoolProperties.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestPoolProperties.java index c9e37b11fd..e8afd5b3b4 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestPoolProperties.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestPoolProperties.java @@ -388,6 +388,46 @@ class TestPoolProperties { "the string is configuration and is not rewritten by a credential clear"); } + @Test + void clearingThePasswordAlsoStripsCredentialAliases() { + PoolProperties properties = new PoolProperties(); + properties.setConnectionProperties("pass=one;pwd=two;password=three;user=u"); + + properties.setPassword(null); + + Assertions.assertNull(properties.getDbProperties().getProperty("password"), + "the password entry must be cleared"); + Assertions.assertNull(properties.getDbProperties().getProperty("pass"), + "the pass alias must not keep authenticating connections"); + Assertions.assertNull(properties.getDbProperties().getProperty("pwd"), + "the pwd alias must not keep authenticating connections"); + Assertions.assertEquals("u", properties.getDbProperties().getProperty("user"), + "non-credential entries must be left alone"); + Assertions.assertEquals("pass=one;pwd=two;password=three;user=u", properties.getConnectionProperties(), + "the configured string is not rewritten by a credential clear"); + } + + @Test + void clearingThePasswordStripsCredentialAliasesSuppliedByADefaultsTable() { + Properties defaults = new Properties(); + defaults.setProperty("pass", "one"); + defaults.setProperty("pwd", "two"); + defaults.setProperty(PoolUtilities.PROP_PASSWORD, "three"); + PoolProperties properties = new PoolProperties(); + properties.setDbProperties(new Properties(defaults)); + + properties.setPassword(null); + + Assertions.assertNull(properties.getDbProperties().getProperty("pass"), + "a defaults-supplied pass alias must be cleared"); + Assertions.assertNull(properties.getDbProperties().getProperty("pwd"), + "a defaults-supplied pwd alias must be cleared"); + Assertions.assertNull(properties.getDbProperties().getProperty(PoolUtilities.PROP_PASSWORD), + "a defaults-supplied password must be cleared"); + Assertions.assertNull(PoolUtilities.clone(properties.getDbProperties()).getProperty("pass"), + "the copy handed to the driver must not materialize a cleared alias"); + } + @Test void clearingCredentialsReachesValuesSuppliedByADefaultsTable() { Properties defaults = new Properties(); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
