Ben Weidig created TAP5-2834:
--------------------------------
Summary: 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
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)
