[
https://issues.apache.org/jira/browse/TAP5-2834?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18092098#comment-18092098
]
ASF subversion and git services commented on TAP5-2834:
-------------------------------------------------------
Commit 3b470b14aa8c9fbc41d8a560ff4e1a67859073b0 in tapestry-5's branch
refs/heads/TAP5-2834 from Ben Weidig
[ https://gitbox.apache.org/repos/asf?p=tapestry-5.git;h=3b470b14a ]
TAP5-2834: PageTesterModule set HMAC passphrase
As tests run in production mode by default, a valid passphrase must be
present.
> 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)
