This is an automated email from the ASF dual-hosted git repository. papegaaij pushed a commit to branch crypt-unification in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 31b7ce12f69e321eb2652cac0c75aa751c727041 Author: Emond Papegaaij <[email protected]> AuthorDate: Fri Jul 3 15:03:26 2026 +0200 WICKET-7190 Bind encrypted pages to their page id via AEAD associated data Expose associated data (AAD) in the crypt API and use it to bind each encrypted page to its id: - ICrypt gains encrypt(byte[], byte[] associatedData) and decrypt(byte[], byte[] associatedData); the no-arg overloads delegate with no associated data. SchemeCrypt authenticates marker || associatedData, so the no-associated-data path is unchanged and URL/cookie ciphertext is unaffected. - CryptingPageStore passes the page id (4 big-endian bytes) as associated data, using the trusted id parameter on getPage and the server-assigned id on addPage. A blob stored for one id can no longer be substituted into another slot by a tamper-capable storage adversary: it fails authentication and is treated as a cache miss. URLs (CryptoMapper) and the remember-me cookie bind no associated data: there is no useful context to add (the session is redundant under a per-session key and breaks URL sharing under an application-wide key; request-specific context would break bookmarkability). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> --- .../wicket/core/util/crypt/SchemeCryptTest.java | 23 +++++++++++++ .../wicket/pageStore/CryptingPageStoreTest.java | 26 +++++++++++++++ .../CryptedUrlWebRequestCodingStrategyTest.java | 4 +-- .../org/apache/wicket/core/util/crypt/ICrypt.java | 39 ++++++++++++++++++++-- .../org/apache/wicket/core/util/crypt/NoCrypt.java | 4 +-- .../apache/wicket/core/util/crypt/SchemeCrypt.java | 27 +++++++++++---- .../apache/wicket/pageStore/CryptingPageStore.java | 15 +++++++-- 7 files changed, 123 insertions(+), 15 deletions(-) diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/core/util/crypt/SchemeCryptTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/core/util/crypt/SchemeCryptTest.java index bde6c01054..a8b77552e3 100644 --- a/wicket-core-tests/src/test/java/org/apache/wicket/core/util/crypt/SchemeCryptTest.java +++ b/wicket-core-tests/src/test/java/org/apache/wicket/core/util/crypt/SchemeCryptTest.java @@ -118,6 +118,29 @@ public class SchemeCryptTest assertNull(crypt(newKey(), scheme).decrypt(enc)); } + @ParameterizedTest + @MethodSource("schemes") + void associatedDataMustMatch(ICryptScheme scheme) + { + SchemeCrypt crypt = crypt(newKey(), scheme); + byte[] plain = "data".getBytes(StandardCharsets.UTF_8); + byte[] aad = { 1, 2, 3, 4 }; + + byte[] enc = crypt.encrypt(plain, aad); + + // only the identical associated data decrypts + assertArrayEquals(plain, crypt.decrypt(enc, aad)); + // a different, absent, or no-arg associated data fails authentication + assertNull(crypt.decrypt(enc, new byte[] { 9, 9, 9, 9 })); + assertNull(crypt.decrypt(enc, null)); + assertNull(crypt.decrypt(enc)); + + // data encrypted without associated data cannot be read as if it had some, and vice versa + byte[] encNoAad = crypt.encrypt(plain); + assertArrayEquals(plain, crypt.decrypt(encNoAad)); + assertNull(crypt.decrypt(encNoAad, aad)); + } + @Test void emptyOrShortInputReturnsNull() { diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/pageStore/CryptingPageStoreTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/pageStore/CryptingPageStoreTest.java index 5c6b763903..214036107d 100644 --- a/wicket-core-tests/src/test/java/org/apache/wicket/pageStore/CryptingPageStoreTest.java +++ b/wicket-core-tests/src/test/java/org/apache/wicket/pageStore/CryptingPageStoreTest.java @@ -119,6 +119,32 @@ public class CryptingPageStoreTest extends WicketTestCase assertNull(store.getPage(context, p)); } + @ParameterizedTest + @MethodSource("schemes") + void pageIsBoundToItsId(ICryptScheme scheme) + { + MockPageStore delegate = new MockPageStore(); + CryptingPageStore store = buildPageStore(scheme, delegate); + JavaSerializer serializer = new JavaSerializer("test"); + + MockPageContext context = new MockPageContext(); + + int n = 1; + int m = 2; + store.addPage(context, new SerializedPage(n, "foo", serializer.serialize(new MockPage(n)))); + + // copy page n's stored ciphertext into slot m + SerializedPage storedN = (SerializedPage) delegate.getPage(context, n); + delegate.addPage(context, new SerializedPage(m, "foo", storedN.getData())); + + // reading slot m must fail: the blob is bound (via associated data) to id n + assertNull(store.getPage(context, m)); + + // page n itself still decrypts + SerializedPage got = (SerializedPage) store.getPage(context, n); + assertEquals(n, ((MockPage) serializer.deserialize(got.getData())).getPageId()); + } + private CryptingPageStore buildPageStore(ICryptScheme scheme, MockPageStore delegate) { tester.getApplication() diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/protocol/http/request/CryptedUrlWebRequestCodingStrategyTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/protocol/http/request/CryptedUrlWebRequestCodingStrategyTest.java index 0c60aeeb11..036c0fce76 100644 --- a/wicket-core-tests/src/test/java/org/apache/wicket/protocol/http/request/CryptedUrlWebRequestCodingStrategyTest.java +++ b/wicket-core-tests/src/test/java/org/apache/wicket/protocol/http/request/CryptedUrlWebRequestCodingStrategyTest.java @@ -109,13 +109,13 @@ class CryptedUrlWebRequestCodingStrategyTest extends WicketTestCase return new ICrypt() { @Override - public byte[] encrypt(byte[] plainBytes) + public byte[] encrypt(byte[] plainBytes, byte[] associatedData) { return plainBytes; } @Override - public byte[] decrypt(byte[] encryptedBytes) + public byte[] decrypt(byte[] encryptedBytes, byte[] associatedData) { return encryptedBytes; } diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/ICrypt.java b/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/ICrypt.java index 76852ddb87..3a84f4551f 100644 --- a/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/ICrypt.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/ICrypt.java @@ -40,19 +40,52 @@ public interface ICrypt * * @param plainBytes * the bytes to encrypt, must not be {@code null} + * @param associatedData + * optional additional data that is authenticated but not encrypted; the identical + * value must be supplied to {@link #decrypt(byte[], byte[])} or decryption fails. + * Use it to bind the ciphertext to its context (e.g. a page id). May be + * {@code null}. * @return the encrypted bytes */ - byte[] encrypt(byte[] plainBytes); + byte[] encrypt(byte[] plainBytes, byte[] associatedData); /** * Decrypt the given bytes. * * @param encryptedBytes * the bytes to decrypt + * @param associatedData + * the same additional data that was supplied to {@link #encrypt(byte[], byte[])}, + * or {@code null} if none was used * @return the decrypted bytes, or {@code null} if the input could not be decrypted for any - * reason (unknown or non-whitelisted scheme, failed authentication, malformed input) + * reason (unknown or non-whitelisted scheme, failed authentication including an + * associated-data mismatch, malformed input) */ - byte[] decrypt(byte[] encryptedBytes); + byte[] decrypt(byte[] encryptedBytes, byte[] associatedData); + + /** + * Encrypt the given bytes without additional associated data. + * + * @param plainBytes + * the bytes to encrypt, must not be {@code null} + * @return the encrypted bytes + */ + default byte[] encrypt(byte[] plainBytes) + { + return encrypt(plainBytes, null); + } + + /** + * Decrypt the given bytes that were encrypted without additional associated data. + * + * @param encryptedBytes + * the bytes to decrypt + * @return the decrypted bytes, or {@code null} if the input could not be decrypted + */ + default byte[] decrypt(byte[] encryptedBytes) + { + return decrypt(encryptedBytes, null); + } /** * Encrypt the given text into a URL-safe (Base64, no padding) {@code String}. diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/NoCrypt.java b/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/NoCrypt.java index 1fa3a992ec..81e4281a08 100644 --- a/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/NoCrypt.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/NoCrypt.java @@ -27,13 +27,13 @@ package org.apache.wicket.core.util.crypt; public class NoCrypt implements ICrypt { @Override - public byte[] encrypt(byte[] plainBytes) + public byte[] encrypt(byte[] plainBytes, byte[] associatedData) { return plainBytes; } @Override - public byte[] decrypt(byte[] encryptedBytes) + public byte[] decrypt(byte[] encryptedBytes, byte[] associatedData) { return encryptedBytes; } diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/SchemeCrypt.java b/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/SchemeCrypt.java index d89a9a2bd6..4602b0867a 100644 --- a/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/SchemeCrypt.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/SchemeCrypt.java @@ -80,13 +80,12 @@ public class SchemeCrypt implements ICrypt } @Override - public byte[] encrypt(byte[] plainBytes) + public byte[] encrypt(byte[] plainBytes, byte[] associatedData) { Args.notNull(plainBytes, "plainBytes"); byte id = encryptionScheme.id(); - byte[] aad = { id }; - byte[] payload = encryptionScheme.encrypt(plainBytes, key, aad, random); + byte[] payload = encryptionScheme.encrypt(plainBytes, key, aad(id, associatedData), random); byte[] result = new byte[payload.length + 1]; result[0] = id; @@ -95,7 +94,7 @@ public class SchemeCrypt implements ICrypt } @Override - public byte[] decrypt(byte[] encryptedBytes) + public byte[] decrypt(byte[] encryptedBytes, byte[] associatedData) { if (encryptedBytes == null || encryptedBytes.length < 1) { @@ -110,8 +109,24 @@ public class SchemeCrypt implements ICrypt return null; } - byte[] aad = { id }; byte[] payload = Arrays.copyOfRange(encryptedBytes, 1, encryptedBytes.length); - return scheme.decrypt(payload, key, aad); + return scheme.decrypt(payload, key, aad(id, associatedData)); + } + + /** + * Builds the AEAD associated data by prefixing the caller-supplied associated data with the + * scheme marker. With no associated data this is just {@code {marker}} (unchanged behavior). + */ + private static byte[] aad(byte marker, byte[] associatedData) + { + if (associatedData == null || associatedData.length == 0) + { + return new byte[] { marker }; + } + + byte[] aad = new byte[associatedData.length + 1]; + aad[0] = marker; + System.arraycopy(associatedData, 0, aad, 1, associatedData.length); + return aad; } } diff --git a/wicket-core/src/main/java/org/apache/wicket/pageStore/CryptingPageStore.java b/wicket-core/src/main/java/org/apache/wicket/pageStore/CryptingPageStore.java index 9860db1a67..e0a714ebcf 100644 --- a/wicket-core/src/main/java/org/apache/wicket/pageStore/CryptingPageStore.java +++ b/wicket-core/src/main/java/org/apache/wicket/pageStore/CryptingPageStore.java @@ -17,6 +17,7 @@ package org.apache.wicket.pageStore; import java.io.Serializable; +import java.nio.ByteBuffer; import java.security.SecureRandom; import javax.crypto.SecretKey; @@ -126,7 +127,9 @@ public class CryptingPageStore extends DelegatingPageStore SerializedPage serializedPage = (SerializedPage) page; byte[] encrypted = serializedPage.getData(); - byte[] decrypted = getCrypt(context).decrypt(encrypted); + // bind the page to the id we are looking up (the trusted method parameter), so a + // blob that was stored for a different id fails authentication + byte[] decrypted = getCrypt(context).decrypt(encrypted, aad(id)); if (decrypted == null) { @@ -152,13 +155,21 @@ public class CryptingPageStore extends DelegatingPageStore SerializedPage serializedPage = (SerializedPage) page; byte[] decrypted = serializedPage.getData(); - byte[] encrypted = getCrypt(context).encrypt(decrypted); + byte[] encrypted = getCrypt(context).encrypt(decrypted, aad(serializedPage.getPageId())); page = new SerializedPage(page.getPageId(), serializedPage.getPageType(), encrypted); getDelegate().addPage(context, page); } + /** + * The associated data binding an encrypted page to its id: the page id as 4 big-endian bytes. + */ + private static byte[] aad(int pageId) + { + return ByteBuffer.allocate(Integer.BYTES).putInt(pageId).array(); + } + private static class SessionData implements Serializable { private static final long serialVersionUID = 1L;
