This is an automated email from the ASF dual-hosted git repository. papegaaij pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 848fefaefb2d3c7719a1af5056414e7bf2fefa9f Author: Emond Papegaaij <[email protected]> AuthorDate: Mon Aug 17 21:21:41 2026 +0200 Document that the URL and cookie crypt is not authenticated The page store crypters say plainly that CBC gives confidentiality without tamper detection, but the crypt used for URLs and cookies never did, on any branch. That is the family behind CryptoMapper and the remember-me cookie in DefaultAuthenticationStrategy, and it is the one an application is most likely to mistake for a security boundary. AESCrypt (AES/CBC/PKCS5Padding) and SunJceCrypt (PBEWithMD5AndDES, which is DES in CBC mode) now state that the ciphertext is malleable, that its integrity is not verified on decryption, and that a successful decryption is therefore no evidence the value was not tampered with. Both point at SECURITY.md and at CryptoMapper for why encrypted URLs are not an access-control mechanism. AESCrypt also notes that passing a different algorithm does not fix this: the cipher is initialised with an IvParameterSpec sized from the block size, so the class is built for IV-based unauthenticated modes and tamper detection needs a different ICrypt, not another algorithm string. The two abstract bases, AbstractCrypt and AbstractJceCrypt, pick no mode themselves, so they carry the shorter form and point subclasses at AEAD. The wicket-util classes refer to CryptoMapper with {@code} rather than {@link}, since wicket-util does not depend on wicket-core. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --- .../java/org/apache/wicket/core/util/crypt/AESCrypt.java | 15 ++++++++++++++- .../apache/wicket/core/util/crypt/AbstractJceCrypt.java | 8 +++++++- .../java/org/apache/wicket/util/crypt/AbstractCrypt.java | 9 ++++++++- .../java/org/apache/wicket/util/crypt/SunJceCrypt.java | 11 ++++++++++- 4 files changed, 39 insertions(+), 4 deletions(-) diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/AESCrypt.java b/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/AESCrypt.java index 59d1d4893e..c1696f33d3 100644 --- a/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/AESCrypt.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/AESCrypt.java @@ -34,7 +34,20 @@ import javax.crypto.spec.IvParameterSpec; /** * AES based {@link ICrypt} encrypt and decrypt strings such as passwords or URL segments. * Based on http://stackoverflow.com/a/992413 - * + * <p> + * <strong>This implementation is not authenticated and provides confidentiality only.</strong> The + * default {@code AES/CBC/PKCS5Padding} produces malleable ciphertext whose integrity is not + * verified on decryption, so someone able to modify an encrypted value may change what it decrypts + * to instead of being detected. Never treat the fact that a value was encrypted as evidence that it + * has not been tampered with, and never use encryption here in place of an authorization check. + * <p> + * Passing a different {@code algorithm} does not change this: the cipher is initialised with an + * {@link javax.crypto.spec.IvParameterSpec} sized from the cipher block size, so this class is + * built for IV-based unauthenticated modes. Tamper detection needs a different {@link ICrypt} + * implementation, not another algorithm string. See {@code SECURITY.md} for the trust assumptions + * Wicket makes here, and {@link org.apache.wicket.core.request.mapper.CryptoMapper} for why + * encrypted URLs are not an access-control mechanism. + * * @see ICrypt */ public class AESCrypt extends AbstractJceCrypt diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/AbstractJceCrypt.java b/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/AbstractJceCrypt.java index 4f5d7b6020..a74c17803e 100644 --- a/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/AbstractJceCrypt.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/util/crypt/AbstractJceCrypt.java @@ -27,7 +27,13 @@ import java.util.Base64; /** * Base class for JCE based ICrypt implementations. - * + * <p> + * This class does not choose a cipher mode, but the implementations shipped with Wicket use + * unauthenticated ones and so provide confidentiality only - see {@link AESCrypt}. A value that + * decrypts without error has therefore not been shown to be unmodified, and callers must not treat + * successful decryption as proof of integrity. Subclasses that want tamper detection should use an + * AEAD cipher mode (such as GCM, GCM-SIV or CCM) rather than layering something on top of an + * unauthenticated mode. */ public abstract class AbstractJceCrypt implements ICrypt { diff --git a/wicket-util/src/main/java/org/apache/wicket/util/crypt/AbstractCrypt.java b/wicket-util/src/main/java/org/apache/wicket/util/crypt/AbstractCrypt.java index c68f124d98..673390a552 100644 --- a/wicket-util/src/main/java/org/apache/wicket/util/crypt/AbstractCrypt.java +++ b/wicket-util/src/main/java/org/apache/wicket/util/crypt/AbstractCrypt.java @@ -29,7 +29,14 @@ import org.slf4j.LoggerFactory; /** * Abstract base class for JCE based ICrypt implementations. - * + * <p> + * This class does not choose a cipher mode, but the implementations shipped with Wicket use + * unauthenticated ones and so provide confidentiality only - see {@link SunJceCrypt}. A value that + * decrypts without error has therefore not been shown to be unmodified, and callers must not treat + * successful decryption as proof of integrity. Subclasses that want tamper detection should use an + * AEAD cipher mode (such as GCM, GCM-SIV or CCM) rather than layering something on top of an + * unauthenticated mode. + * * @author Juergen Donnerstag */ public abstract class AbstractCrypt implements ICrypt diff --git a/wicket-util/src/main/java/org/apache/wicket/util/crypt/SunJceCrypt.java b/wicket-util/src/main/java/org/apache/wicket/util/crypt/SunJceCrypt.java index 52be417561..d7fa3adad8 100644 --- a/wicket-util/src/main/java/org/apache/wicket/util/crypt/SunJceCrypt.java +++ b/wicket-util/src/main/java/org/apache/wicket/util/crypt/SunJceCrypt.java @@ -37,7 +37,16 @@ import org.apache.wicket.util.lang.Args; * implementation is based around Sun's security providers and uses the <a * href="http://www.ietf.org/rfc/rfc2898.txt";>PBEWithMD5AndDES</a> method to encrypt and decrypt the * data. - * + * <p> + * <strong>This implementation is not authenticated and provides confidentiality only.</strong> + * {@code PBEWithMD5AndDES} is DES in CBC mode: the ciphertext is malleable and its integrity is not + * verified on decryption, so someone able to modify an encrypted value may change what it decrypts + * to instead of being detected. Never treat the fact that a value was encrypted as evidence that it + * has not been tampered with, and never use encryption here in place of an authorization check. + * Passing another {@code cryptMethod} does not by itself change this. See {@code SECURITY.md} for + * the trust assumptions Wicket makes here, and {@code CryptoMapper} for why encrypted URLs are not + * an access-control mechanism. + * * @author Juergen Donnerstag */ public class SunJceCrypt extends AbstractCrypt
