TAP5-2834: Fail startup in production mode on weak/empty HMAC passphrase

tapestry-core/src/main/java/org/apache/tapestry5/SymbolConstants.java

@@ -397,9 +397,13 @@ public class SymbolConstants
 
     /**
      * A passphrase used as the basis of hash-based message authentication (HMAC) for any object stream data stored on
-     * the client.  The default phrase is the empty string, which will result in a logged runtime <em>error</em>.
-     * You should configure this to a reasonable value (longer is better) and ensure that all servers in your cluster
-     * share the same value (configuring this in code, rather than the command line, is preferred).
+     * the client. The passphrase must be at least 20 characters long (aligned with the RFC 2104 SHA-1 digest length).
+     * You should configure this to a strong, random value and ensure that all servers in your cluster share the same
+     * value (configuring this in code, rather than on the command line, is preferred).
+     *
+     * <p>In <strong>production mode</strong>, a missing or too-short passphrase causes a startup exception.
+     * In <strong>development mode</strong>, a warning is logged and a random per-startup fallback key is used instead,
+     * meaning client data will not survive server restarts.</p>
      *
      * @see org.apache.tapestry5.services.ClientDataEncoder
      * @since 5.3.6

tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ClientDataEncoderImpl.java

@@ -1,4 +1,4 @@
-// Copyright 2009, 2012 The Apache Software Foundation
+// Copyright 2009, 2012, 2026 The Apache Software Foundation
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -14,9 +14,9 @@
 
 package org.apache.tapestry5.internal.services;
 
+import org.apache.commons.lang3.StringUtils;
 import org.apache.tapestry5.SymbolConstants;
 import org.apache.tapestry5.alerts.AlertManager;
-import org.apache.tapestry5.http.internal.TapestryHttpInternalConstants;
 import org.apache.tapestry5.internal.TapestryInternalUtils;
 import org.apache.tapestry5.internal.util.Base64InputStream;
 import org.apache.tapestry5.internal.util.MacOutputStream;
@@ -30,43 +30,53 @@
 import java.io.BufferedInputStream;
 import java.io.IOException;
 import java.io.ObjectInputStream;
-import java.io.UnsupportedEncodingException;
+import java.nio.charset.StandardCharsets;
 import java.security.Key;
 import java.security.MessageDigest;
+import java.util.UUID;
 import java.util.zip.GZIPInputStream;
 
 public class ClientDataEncoderImpl implements ClientDataEncoder
 {
+    private static final int MIN_PASSPHRASE_LENGTH = 20;
+
     private final URLEncoder urlEncoder;
 
     private final Key hmacKey;
 
-    public ClientDataEncoderImpl(URLEncoder urlEncoder, @Symbol(SymbolConstants.HMAC_PASSPHRASE) String passphrase,
+    public ClientDataEncoderImpl(URLEncoder urlEncoder,
+                                 @Symbol(SymbolConstants.HMAC_PASSPHRASE) String passphrase,
                                  Logger logger,
-                                 @Symbol(TapestryHttpInternalConstants.TAPESTRY_APP_PACKAGE_PARAM)
-                                 String applicationPackageName, AlertManager alertManager) throws UnsupportedEncodingException
+                                 AlertManager alertManager,
+                                 @Symbol(SymbolConstants.PRODUCTION_MODE) boolean productionMode)
     {
         this.urlEncoder = urlEncoder;
 
-        if (passphrase.equals(""))
+        // TAP5-2834: Also check for minimum length
+        if (StringUtils.isBlank(passphrase) || passphrase.length() < MIN_PASSPHRASE_LENGTH)
         {
-            String message = String.format("The symbol '%s' has not been configured. " +
-                    "This is used to configure hash-based message authentication of Tapestry data stored in forms, or in the URL. " +
-                    "You application is less secure, and more vulnerable to denial-of-service attacks, when this symbol is not configured.",
-                    SymbolConstants.HMAC_PASSPHRASE);
+            String message = String.format(
+                    "SymbolConstants.HMAC_PASSPHRASE '%s' has not been configured or is too short (minimum %d characters). " +
+                    "This is used to configure hash-based message authentication of Tapestry data stored in forms or in the URL. " +
+                    "Your application is less secure and more vulnerable to denial-of-service attacks when this symbol is not properly configured.",
+                    SymbolConstants.HMAC_PASSPHRASE, MIN_PASSPHRASE_LENGTH);
 
-            // Now to really get the attention of the developer!
+            if (productionMode)
+            {
+                throw new RuntimeException(message);
+            }
 
             alertManager.error(message);
-
             logger.error(message);
 
-            // Override the blank parameter to set a default value. Use the application package name,
-            // which is justly slightly more secure than having a fixed default.
-            passphrase = applicationPackageName;
+            // TAP5-2834: No longer fall back to application package, as it's easier to guess/deduct.
+            // As we disallow an empty passphrase in production, this should only become an issue
+            // if run in non-production in a clustered environment. and even then, the easy fix
+            // will be configuring a custom passphrase.
+            passphrase = UUID.randomUUID().toString();
         }
 
-        hmacKey = new SecretKeySpec(passphrase.getBytes("UTF8"), "HmacSHA1");
+        hmacKey = new SecretKeySpec(passphrase.getBytes(StandardCharsets.UTF_8), "HmacSHA1");
     }
 
     public ClientDataSink createSink()
@@ -127,7 +137,7 @@ private void validateHMAC(String storedHmacResult, Base64InputStream b64in) thro
 
         String actual = macOs.getResult();
 
-        if (!MessageDigest.isEqual(storedHmacResult.getBytes(), actual.getBytes()))
+        if (!MessageDigest.isEqual(storedHmacResult.getBytes(StandardCharsets.UTF_8), actual.getBytes(StandardCharsets.UTF_8)))
         {
             throw new IOException("Client data associated with the current request appears to have been tampered with " +
                     "(the HMAC signature does not match).");

tapestry-core/src/main/java/org/apache/tapestry5/internal/test/PageTesterModule.java

@@ -12,6 +12,7 @@
 
 package org.apache.tapestry5.internal.test;
 
+import org.apache.tapestry5.SymbolConstants;
 import org.apache.tapestry5.commons.MappedConfiguration;
 import org.apache.tapestry5.commons.ObjectLocator;
 import org.apache.tapestry5.commons.OrderedConfiguration;
@@ -26,7 +27,10 @@
 import org.apache.tapestry5.ioc.ServiceBinder;
 import org.apache.tapestry5.ioc.annotations.Contribute;
 import org.apache.tapestry5.ioc.annotations.Local;
+import org.apache.tapestry5.ioc.services.ApplicationDefaults;
+import org.apache.tapestry5.ioc.services.FactoryDefaults;
 import org.apache.tapestry5.ioc.services.ServiceOverride;
+import org.apache.tapestry5.ioc.services.SymbolProvider;
 import org.apache.tapestry5.services.MarkupRendererFilter;
 import org.apache.tapestry5.test.PageTester;
 
@@ -86,4 +90,11 @@ public static void contributeMarkupRenderer(OrderedConfiguration<MarkupRendererF
     {
         configuration.addInstance("CaptureRenderedDocument", CaptureRenderedDocument.class, "before:DocumentLinker");
     }
+
+    @Contribute(SymbolProvider.class)
+    @ApplicationDefaults
+    public static void contributeApplicationDefaults(MappedConfiguration<String, Object> configuration)
+    {
+        configuration.add(SymbolConstants.HMAC_PASSPHRASE, "PageTester default passphrase for testing");
+    }
 }