[jira] [Assigned] (TAP5-2834) Fail startup in production mode when tapestry.hmac-passphrase is not configured or too weak

["Ben Weidig (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/TAP5-2834?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ben Weidig reassigned TAP5-2834:
--------------------------------

    Assignee: Ben Weidig

> Fail startup in production mode when tapestry.hmac-passphrase is not configured or too weak
> -------------------------------------------------------------------------------------------
>
>                 Key: TAP5-2834
>                 URL: https://issues.apache.org/jira/browse/TAP5-2834
>             Project: Tapestry 5
>          Issue Type: Improvement
>          Components: tapestry-core
>            Reporter: Ben Weidig
>            Assignee: Ben Weidig
>            Priority: Major
>
> h2. Description
> {{ClientDataEncoderImpl}} ([tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ClientDataEncoderImpl.java]) is responsible for HMAC-signing client-side data (form/URL state) using the {{SymbolConstants.HMAC_PASSPHRASE}} symbol as the signing key.
> When the passphrase symbol is left unconfigured (empty string), the current behavior is "loud, but non-failure":
>  * An {*}error{*}-level message is logged via SLF4J.
>  * The same message is pushed to the framework's {{{}AlertManager{}}}, which surfaces it client-side *if* the related component/mechanism is actually used on the page.
>  * The framework then falls back to using the application's root package name as the passphrase and continues starting up normally.
> Today this check only covers the *empty-string* case ({{{}passphrase.equals(""){}}}).
> A passphrase consisting only of whitespace (e.g. {{{}" "{}}}) is not blank by that check and passes straight through as the HMAC key, even though no real entropy was provided.
> Short or weak but non-blank/non-whitespace passphrases (e.g. {{{}"x"{}}}, a 4-digit PIN, or a short package name like {{{}com.foo{}}}) also pass through today with *no* validation, no log, and no alert.
> This is arguably the bigger real-world gap, since it produces no signal at all.
> Per RFC 2104, an HMAC-SHA1 key should be at least as long as the hash output (20 bytes).
> Keys longer than the block size (64 bytes) add no additional security.
> So a sane minimum is {*}20 characters{*}.
> The fallback passphrase is the application package name, e.g. {{{}com.example.myapp{}}}.
> It's fixed, guessable, publicly-derivable value, so not a secret.
> It does *not* currently get randomized in non-production mode.
> The log/alert is the only signal that anything is wrong, and it is easy to miss outside of active development testing.
> h2. Proposed Solution
>  * *Non-production mode:*
>  ** Keep the current "loud" behavior (error log + {{AlertManager}} alert) so developers are clearly warned, but do not block startup, to keep a fast local dev loop.
>  ** Extend this same warn-and-fallback treatment to passphrases that are non-blank but shorter than the minimum length.
>  ** Fallback should be randomized between registry startups.
>  * {*}Production mode{*}:
>  ** Escalate to a hard *startup failure* (thrown exception) instead of silently falling back to a weak default
>  ** Disallow *both* the blank case and the too-short case. A production deployment running with HMAC disabled, defaulted, or under-length should never reach end users undetected.
> h2. Proposed Changes
>  * Inject {{@Symbol(SymbolConstants.PRODUCTION_MODE)}} (as {{{}boolean{}}}) into {{{}ClientDataEncoderImpl{}}}'s constructor.
>  * Introduce a minimum-length constant (proposed: {*}20 characters{*}, matching the SHA-1 digest length per RFC 2104).
>  * Replace the current {{passphrase.equals("")}} check with {{{}passphrase.isBlank() || passphrase.length() < MIN_LENGTH{}}}, so whitespace-only passphrases (e.g. {{{}" "{}}}) are treated the same as truly empty ones, not as valid keys.
>  * When the passphrase is blank/whitespace-only or under the minimum length:
>  ** If *not* production mode: keep existing log + alert behavior (message updated to also cover the "blank/whitespace" and "too short" cases), then fall back to an UUID.
>  ** If production mode: throw a {{RuntimeException}} (or a dedicated {{{}TapestryException{}}}) describing the missing/weak {{tapestry.hmac-passphrase}} symbol and abort service construction, preventing the application from starting.
>  * Update/add documentation for {{SymbolConstants.HMAC_PASSPHRASE}} to mention the minimum length requirement and the production-mode startup failure.
> h2. Risks
>  * *Breaking change for existing production deployments:* Any application currently running in production without {{tapestry.hmac-passphrase}} configured will fail to start after upgrading, where previously it started silently with a weak key. This is the intended effect, but should be called out prominently in release notes as a breaking change, with clear remediation steps (set the symbol).
>  * *Testing/CI impact:* Test harnesses or example apps that run with production mode enabled but no passphrase configured will start failing. We need to audit Tapestry's own test suite and archetypes for this combination before merging.
>  * *Severity of exception message:* The thrown exception must clearly name the missing symbol and link to documentation, otherwise this trades a recoverable warning for a confusing hard failure for users unfamiliar with the symbol.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)