[
https://issues.apache.org/jira/browse/WICKET-7190?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18101688#comment-18101688
]
ASF GitHub Bot commented on WICKET-7190:
----------------------------------------
papegaaij opened a new pull request, #1536:
URL: https://github.com/apache/wicket/pull/1536
Wicket had three overlapping crypt abstractions —
`org.apache.wicket.util.crypt.ICrypt` (URL encryption, "remember me"),
`org.apache.wicket.pageStore.crypt.ICrypter` (page store), and the
`ICryptFactory` hierarchy — with different guarantees. Notably the page store
default, `DefaultCrypter`, was AES-256-CBC: unauthenticated, so stored bytes
were confidential but malleable, and tampered ciphertext still reached the
deserializer.
This replaces all of it with one authenticated abstraction.
> **Base branch:** this targets `document-security-model` (#1535), not
`master` — the first commit edits `SECURITY.md`, which that branch introduces.
It will retarget to `master` automatically once the base merges.
### What changes
**One interface.** `org.apache.wicket.core.util.crypt.ICrypt` is now the
single abstraction, used by the page store, `CryptoMapper` and the
authentication cookie alike. It operates on `byte[]` with an optional
associated-data parameter, and provides the URL-safe Base64 `String` layer as
default methods. `decrypt` returns `null` on *any* failure — unknown scheme,
failed authentication, malformed input — so callers uniformly treat
undecryptable data as absent rather than catching exceptions.
**Authenticated by default, and self-describing.** `SchemeCrypt` writes
`marker(1) || scheme-payload`, where the marker is the id of the `ICryptScheme`
that produced the payload. Encryption always uses the configured scheme;
decryption looks the marker up in a whitelist and refuses anything not on it.
The marker is authenticated as associated data, so it cannot be altered to
force a weaker scheme — together that gives downgrade protection, and a
documented migration path (whitelist the old scheme while the new one becomes
the encryption scheme, then drop it once data is rewritten).
**Two shipped schemes**, both AEAD:
- `AesGcmCryptScheme` — JDK-native AES-256-GCM. The new default; no extra
dependencies.
- `AesGcmSivCryptScheme` — AES-256-GCM-SIV, nonce-misuse resistant. Requires
Bouncy Castle.
**Scheme and key source are separated.** An `ICryptScheme` decides *how*
data is encrypted and generates the keys it consumes; an `ICryptFactory`
decides only *where* the key lives — `KeyInSessionCryptFactory` (per session,
the default) or `ApplicationKeyCryptFactory` (application-wide, for stateless
deployments).
**Encrypted pages are bound to their page id.** `CryptingPageStore` passes
the page id as associated data, so a stored page cannot be replayed as a
different one.
### Behavioural fix outside the crypt package
`AbstractFileUploadResource` validated its signed upload settings by
re-encrypting the expected values and comparing the resulting *ciphertext*.
That only worked because the previous default was deterministic PBE.
`SchemeCrypt` uses a fresh nonce per message, as an AEAD mode must, so the
comparison could never succeed and every `FileUploadToResourceField` upload
would have been rejected. It now decrypts the token and compares the settings
it carries; the authentication tag is what makes the token unforgeable, and
`FileUploadToResourceFieldSecurityTest` still rejects tampered limits.
### Compatibility — this is a Wicket 11 API break
Removed with no replacement: `SunJceCrypt`, `AbstractCrypt`, `TrivialCrypt`,
`CryptFactoryCachingDecorator`, `AESCrypt`, `AbstractJceCrypt`,
`KeyInSessionSunJceCryptFactory`, `AbstractKeyInSessionCryptFactory`,
`DefaultCrypter`, `GCMSIVCrypter`, `ICrypter`. `NoCrypt`, `NoCryptFactory` and
`ICryptFactory` move to `org.apache.wicket.core.util.crypt`. OpenRewrite
recipes for the moves and notes on the removals are in `wicket-migration`.
Existing ciphertext does not decrypt under the new schemes. In practice that
means encrypted URLs and page store entries from a previous version are treated
as expired, and "remember me" cookies are invalidated.
### Testing
- Full `wicket-core-tests` suite: **2331 tests, 0 failures**.
- Full reactor `clean test-compile` passes.
- New coverage: `SchemeCryptTest` (19), `CryptFactoryTest` (3), plus
extended `CryptoMapperTest` and `CryptingPageStoreTest` — round-trips,
associated-data mismatch, whitelist refusal, marker tampering.
**Known red:** `japicmp` will fail on the removals above and needs a Wicket
11 baseline before CI is green.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
> Unified cryptography API
> ------------------------
>
> Key: WICKET-7190
> URL: https://issues.apache.org/jira/browse/WICKET-7190
> Project: Wicket
> Issue Type: New Feature
> Components: wicket-core
> Reporter: Emond Papegaaij
> Assignee: Emond Papegaaij
> Priority: Major
> Fix For: 11.0.0
>
>
> The current cryptography API in Wicket is split over {{ICrypt}} and
> {{ICrypter}}. Both APIs serve a similar purpose but for different subsystems.
> The goal is to unify these APIs and to allow the implementation of more
> modern ciphers, with AEAD support. Also, the documentation on this API and
> its implementations must be improved, clearly stating what it does and what
> it does not deliver.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
