This is an automated email from the ASF dual-hosted git repository. papegaaij pushed a commit to branch wicket-9.x in repository https://gitbox.apache.org/repos/asf/wicket.git
commit df28f7505a79108b5ca79e828d1324b83ca87c49 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 66be7bf2fb..1c26f9e40c 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 da8155ec1a..4f1c40fa22 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 09dce341ab..17ce328b74 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
