(plc4x) 01/02: fix(plc4j/spi): mask the pre-shared key when a connection string is logged

[sr...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

sruehl pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/plc4x.git

commit d57ee5c59427fedd7e121aaa704df0c0660baa23
Author: Sebastian Rühl <sr...@apache.org>
AuthorDate: Mon Aug 24 16:51:44 2026 +0200

    fix(plc4j/spi): mask the pre-shared key when a connection string is logged
    
    A TLS-PSK connection's credential is the key itself, and the pattern that masks
    secrets before a connection string reaches the logs did not reach it: "psk-key"
    carries none of password, passwd, secret or token. So it was logged in full.
    
    Named in full rather than by matching "key" or "psk", which would have taken
    key-store-file, keystore, key-store-type, generated-key-size, log-session-keys and
    psk-identity with it - a path, a path, a store type, a number, a boolean and an
    identifier. None of those is a secret, masking them protects nothing, and it costs
    whoever is reading the log the diagnosis; the identity most of all, because it says
    which key was refused. There is now a test holding that line, so widening this
    later cannot quietly undo it.
    
    "passphrase" goes in too. No parameter uses it today, but it is the conventional
    name for the same thing and this pattern is what stands between a new one and the
    logs.
    
    This closes the disclosure without the annotation-driven mechanism that was put to
    the dev list, which stays worth doing: a denylist of names in one module cannot
    know about a parameter added in another, which is how this one was missed.
---
 .../apache/plc4x/java/spi/drivers/DriverBase.java  | 13 +++++++---
 .../plc4x/java/spi/drivers/DriverBaseTest.java     | 29 ++++++++++++++++++++++
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/plc4j/spi/drivers/src/main/java/org/apache/plc4x/java/spi/drivers/DriverBase.java b/plc4j/spi/drivers/src/main/java/org/apache/plc4x/java/spi/drivers/DriverBase.java
index 5c3039a3c8..e4621b1223 100644
--- a/plc4j/spi/drivers/src/main/java/org/apache/plc4x/java/spi/drivers/DriverBase.java
+++ b/plc4j/spi/drivers/src/main/java/org/apache/plc4x/java/spi/drivers/DriverBase.java
@@ -84,11 +84,18 @@ public abstract class DriverBase implements PlcDriver {
 
     /**
      * Matches the value of connection-string query parameters that carry secrets so they can be
-     * masked before logging. Covers any parameter whose name contains "password", "passwd", "secret"
-     * or "token" (optionally with a transport prefix like "tls.").
+     * masked before logging. Covers any parameter whose name contains "password", "passwd",
+     * "secret", "token", "psk-key" or "passphrase" (optionally with a transport prefix like
+     * "tls.").
+     *
+     * <p>"psk-key" is named in full rather than matching "key" or "psk" alone, which would take
+     * "key-store-file", "key-store-type", "generated-key-size", "log-session-keys" and
+     * "psk-identity" with it. Those are a path, a store type, a number, a boolean and an
+     * identifier - masking them protects nothing and costs an operator the diagnosis, the identity
+     * most of all, since it says which key was refused.</p>
      */
     private static final Pattern SECRET_PARAM_PATTERN = Pattern.compile(
-        "(?i)([?&][^=&]*(?:password|passwd|secret|token)[^=&]*=)[^&]*");
+        "(?i)([?&][^=&]*(?:password|passwd|secret|token|psk-key|passphrase)[^=&]*=)[^&]*");
 
     private static final Logger log = LoggerFactory.getLogger(DriverBase.class);
 
diff --git a/plc4j/spi/drivers/src/test/java/org/apache/plc4x/java/spi/drivers/DriverBaseTest.java b/plc4j/spi/drivers/src/test/java/org/apache/plc4x/java/spi/drivers/DriverBaseTest.java
index d1b83c8467..f76bd4c7f1 100644
--- a/plc4j/spi/drivers/src/test/java/org/apache/plc4x/java/spi/drivers/DriverBaseTest.java
+++ b/plc4j/spi/drivers/src/test/java/org/apache/plc4x/java/spi/drivers/DriverBaseTest.java
@@ -72,6 +72,35 @@ public class DriverBaseTest {
             assertEquals(url, DriverBase.redactSecrets(url));
         }
 
+        @Test
+        @DisplayName("masks the pre-shared key")
+        void masksPreSharedKey() {
+            // The key itself is the credential for a TLS-PSK connection, and nothing in the
+            // pattern reached it: it carries none of password, passwd, secret or token.
+            assertEquals(
+                "plc4x:tls-psk://host?tls-psk.psk-identity=plc4x&tls-psk.psk-key=***",
+                DriverBase.redactSecrets(
+                    "plc4x:tls-psk://host?tls-psk.psk-identity=plc4x&tls-psk.psk-key=0011deadbeef"));
+        }
+
+        @Test
+        @DisplayName("masks a passphrase")
+        void masksPassphrase() {
+            assertEquals("plc4x://host?passphrase=***",
+                DriverBase.redactSecrets("plc4x://host?passphrase=open sesame"));
+        }
+
+        @Test
+        @DisplayName("leaves the things named like keys that are not keys")
+        void leavesKeyShapedNonSecretsUntouched() {
+            // Masking these would cost an operator the diagnosis and protect nothing: a path, a
+            // store type, a key size, a boolean, and the identity that says *which* key failed.
+            String url = "plc4x:tls://host?key-store-file=/etc/plc4x/client.p12"
+                + "&key-store-type=pkcs12&generated-key-size=2048&log-session-keys=false"
+                + "&tls-psk.psk-identity=plc4x&allow-insecure-credentials=false";
+            assertEquals(url, DriverBase.redactSecrets(url));
+        }
+
         @Test
         @DisplayName("leaves a connection string without parameters untouched")
         void leavesNoParamStringUntouched() {