This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 516d9f7a92fde3d50c83536cd13ccd2aba09ba14 Author: opencode <[email protected]> AuthorDate: Thu Nov 5 21:02:35 2026 +0100 Mask every credential-shaped database property key cloneWithoutPassword removed only the exact lower-case password key. The DataSource and JMX DbProperties getters expose the resulting copy, while the string maskers and PoolProperties.isCredentialKey already treat pass, pwd and case variants as credentials, so a credential stored under one of those spellings stayed readable through the management surface. Centralize the credential-key predicate in PoolUtilities, ignoring case and surrounding whitespace, and use it to remove every matching entry from the clone, including entries materialized from a defaults table and entries with non-String values. PoolProperties delegates to the same predicate so the two policies cannot drift apart. Add tests covering pass, pwd, PASSWORD, surrounding whitespace and defaults-backed entries. --- .../apache/tomcat/jdbc/pool/DataSourceProxy.java | 8 +++-- .../apache/tomcat/jdbc/pool/PoolProperties.java | 8 ++--- .../org/apache/tomcat/jdbc/pool/PoolUtilities.java | 37 ++++++++++++++++++---- .../tomcat/jdbc/pool/jmx/ConnectionPool.java | 9 +++--- .../apache/tomcat/jdbc/pool/TestPoolUtilities.java | 35 ++++++++++++++++++++ 5 files changed, 80 insertions(+), 17 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DataSourceProxy.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DataSourceProxy.java index 7fe709bbde..84e37a34cb 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DataSourceProxy.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DataSourceProxy.java @@ -1165,9 +1165,11 @@ public class DataSourceProxy implements PoolConfiguration { /** - * Returns a copy of the database properties with the password removed. This object doubles as its own JMX/bean - * surface and {@link #setPassword(String)} puts the password into the database properties, so returning them as - * they are would expose the plaintext password through a getter while {@link #getPassword()} is explicitly masked - + * Returns a copy of the database properties with the credential entries removed. A credential entry is one whose + * key is {@code password}, {@code pass} or {@code pwd}, ignoring case and surrounding whitespace. This object + * doubles as its own JMX/bean surface and {@link #setPassword(String)} puts the password into the database + * properties, so returning them as they are would expose the plaintext password through a getter while + * {@link #getPassword()} is explicitly masked - * the same protection the pool's own MBean * ({@link org.apache.tomcat.jdbc.pool.jmx.ConnectionPool#getDbProperties()}) already applies. A configuration whose * database properties are {@code null} - a state {@link #setPassword(String)} and {@link #setUsername(String)} 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 0775899afa..2a6a58779e 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 @@ -1617,16 +1617,16 @@ public class PoolProperties implements PoolConfiguration, Cloneable, Serializabl /** * Whether {@code key} names a credential, ignoring case and surrounding - * whitespace. + * whitespace. The predicate is shared with the masked property copies + * {@link PoolUtilities#cloneWithoutPassword(Properties)} builds, so the + * two policies cannot drift apart. * * @param key the extracted key * * @return {@code true} when the key is one of the recognised credential keys */ private static boolean isCredentialKey(String key) { - return DataSourceFactory.PROP_PASSWORD.equalsIgnoreCase(key) - || "pass".equalsIgnoreCase(key) - || "pwd".equalsIgnoreCase(key); + return PoolUtilities.isCredentialKey(key); } /** diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolUtilities.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolUtilities.java index b797ac16f3..85a0ed6028 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolUtilities.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolUtilities.java @@ -89,22 +89,47 @@ public class PoolUtilities { return c; } + /** + * Whether {@code key} names a credential, ignoring case and surrounding whitespace. The drivers accept + * {@code pass} and {@code pwd} as aliases of {@code password}, so a masked copy has to remove the same spellings + * the string maskers treat as credentials. + * + * @param key the property key to test, possibly {@code null} + * + * @return {@code true} when the key is one of the recognised credential keys + */ + static boolean isCredentialKey(String key) { + if (key == null) { + return false; + } + String candidate = key.trim(); + return PROP_PASSWORD.equalsIgnoreCase(candidate) || "pass".equalsIgnoreCase(candidate) + || "pwd".equalsIgnoreCase(candidate); + } + /** * Creates a copy of the given properties as {@link #clone(Properties)} - * does, with the password entry removed. The copy carries no defaults - * table, so the removal also reaches a password the source only resolved - * through its defaults - a reference copy would let the remove walk past - * the defaults table and leave the password resolvable. + * does, with every credential entry removed. A credential entry is one + * whose key is recognised by {@link #isCredentialKey(String)}: + * {@code password}, {@code pass} and {@code pwd}, ignoring case and + * surrounding whitespace. The copy carries no defaults table, so the + * removal also reaches a credential the source only resolved through its + * defaults - a reference copy would let the remove walk past the defaults + * table and leave the credential resolvable. * * @param p the properties to clone, not {@code null} * - * @return a new Properties instance with all entries except the password + * @return a new Properties instance with all entries except the credentials * * @throws NullPointerException if the source is {@code null} */ public static Properties cloneWithoutPassword(Properties p) { Properties result = clone(p); - result.remove(PROP_PASSWORD); + for (Object key : result.keySet().toArray()) { + if (key instanceof String && isCredentialKey((String) key)) { + result.remove(key); + } + } return result; } } diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/jmx/ConnectionPool.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/jmx/ConnectionPool.java index cd4c22c2af..c182557b84 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/jmx/ConnectionPool.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/jmx/ConnectionPool.java @@ -482,10 +482,11 @@ public class ConnectionPool extends NotificationBroadcasterSupport implements Co } /** - * Returns a copy of the database properties with the password removed, so the JMX surface does not expose the - * plaintext password while {@link #getPassword()} is explicitly masked. A configuration whose database properties - * are {@code null} - a state {@link PoolConfiguration#setDbProperties(Properties)} accepts - has nothing to mask, - * so this returns {@code null}. {@inheritDoc} + * Returns a copy of the database properties with the credential entries removed, so the JMX surface does not + * expose the plaintext password while {@link #getPassword()} is explicitly masked. A credential entry is one whose + * key is {@code password}, {@code pass} or {@code pwd}, ignoring case and surrounding whitespace. A configuration + * whose database properties are {@code null} - a state {@link PoolConfiguration#setDbProperties(Properties)} + * accepts - has nothing to mask, so this returns {@code null}. {@inheritDoc} * <p> * The copy is a display value, and it looks complete: nothing in it says that the pool's own set carries a pair * this one withholds. That matters because this attribute is also writable and its write is the whole set - diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestPoolUtilities.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestPoolUtilities.java index af2f2d1a11..7bd99b6f1a 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestPoolUtilities.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestPoolUtilities.java @@ -16,6 +16,7 @@ */ package org.apache.tomcat.jdbc.pool; +import java.util.Collections; import java.util.Properties; import org.junit.jupiter.api.Assertions; @@ -149,6 +150,40 @@ class TestPoolUtilities { Assertions.assertFalse(masked.containsKey(PoolUtilities.PROP_PASSWORD)); } + @Test + void cloneWithoutPasswordMasksEveryCredentialKeySpelling() { + Properties source = new Properties(); + source.setProperty("sslmode", "verify-full"); + source.setProperty(PoolUtilities.PROP_PASSWORD, "secret1"); + source.setProperty("pass", "secret2"); + source.setProperty("pwd", "secret3"); + source.setProperty("PASSWORD", "secret4"); + source.setProperty(" Password ", "secret5"); + + Properties masked = PoolUtilities.cloneWithoutPassword(source); + + Assertions.assertEquals(Collections.singleton("sslmode"), + masked.stringPropertyNames(), + "every credential-shaped key must be removed, whatever its spelling"); + } + + @Test + void cloneWithoutPasswordMasksCredentialKeysResolvedFromDefaults() { + Properties defaults = new Properties(); + defaults.setProperty("pass", "secret1"); + defaults.setProperty("pwd", "secret2"); + defaults.setProperty("PASSWORD", "secret3"); + defaults.setProperty("sslmode", "verify-full"); + Properties source = new Properties(defaults); + + Properties masked = PoolUtilities.cloneWithoutPassword(source); + + Assertions.assertEquals(Collections.singleton("sslmode"), + masked.stringPropertyNames(), + "a credential the source resolves through its defaults must not survive" + + " into the tableless copy"); + } + @Test void cloneWithoutPasswordMasksADirectlyPresentPassword() { Properties source = new Properties(); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
