From 6335381c978ce83c9c15bd3c349f32d1bed72d4f Mon Sep 17 00:00:00 2001 From: lhazlewood <121180+lhazlewood@users.noreply.github.com> Date: Sun, 28 Jan 2024 13:17:53 -0800 Subject: [PATCH] PBES2 decryption maximum iterations (#911) Ensured there is an upper bound (maximum) iterations enforced for PBES2 decryption to help mitigate potential DoS attacks. Many thanks to Jingcheng Yang and Jianjun Chen from Sichuan University and Zhongguancun Lab for their work on this! --- CHANGELOG.md | 3 ++ .../impl/security/Pbes2HsAkwAlgorithm.java | 16 ++++++++- .../security/Pbes2HsAkwAlgorithmTest.groovy | 33 +++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8230a11e9..33ce2fb91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,9 @@ This release also: [Issue 901](https://github.com/jwtk/jjwt/issues/901). * Ensures that Secret JWKs for HMAC-SHA algorithms with `k` sizes larger than the algorithm minimum can be parsed/used as expected. See [Issue #905](https://github.com/jwtk/jjwt/issues/905) +* Ensures there is an upper bound (maximum) iterations enforced for PBES2 decryption to help mitigate potential DoS + attacks. Many thanks to Jingcheng Yang and Jianjun Chen from Sichuan University and Zhongguancun Lab for their + work on this. See [PR 911](https://github.com/jwtk/jjwt/pull/911). * Fixes various typos in documentation and JavaDoc. Thanks to those contributing pull requests for these! ### 0.12.3 diff --git a/impl/src/main/java/io/jsonwebtoken/impl/security/Pbes2HsAkwAlgorithm.java b/impl/src/main/java/io/jsonwebtoken/impl/security/Pbes2HsAkwAlgorithm.java index 064b545d3..796c78c8e 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/security/Pbes2HsAkwAlgorithm.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/security/Pbes2HsAkwAlgorithm.java @@ -16,9 +16,11 @@ package io.jsonwebtoken.impl.security; import io.jsonwebtoken.JweHeader; +import io.jsonwebtoken.UnsupportedJwtException; import io.jsonwebtoken.impl.DefaultJweHeader; import io.jsonwebtoken.impl.lang.Bytes; import io.jsonwebtoken.impl.lang.CheckedFunction; +import io.jsonwebtoken.impl.lang.Parameter; import io.jsonwebtoken.impl.lang.ParameterReadable; import io.jsonwebtoken.impl.lang.RequiredParameterReader; import io.jsonwebtoken.lang.Assert; @@ -50,11 +52,13 @@ public class Pbes2HsAkwAlgorithm extends CryptoAlgorithm implements KeyAlgorithm "[JWA RFC 7518, Section 4.8.1.2](https://www.rfc-editor.org/rfc/rfc7518.html#section-4.8.1.2) " + "recommends password-based-encryption iterations be greater than or equal to " + MIN_RECOMMENDED_ITERATIONS + ". Provided: "; + private static final double MAX_ITERATIONS_FACTOR = 2.5; private final int HASH_BYTE_LENGTH; private final int DERIVED_KEY_BIT_LENGTH; private final byte[] SALT_PREFIX; private final int DEFAULT_ITERATIONS; + private final int MAX_ITERATIONS; private final KeyAlgorithm wrapAlg; private static byte[] toRfcSaltPrefix(byte[] bytes) { @@ -106,6 +110,7 @@ protected Pbes2HsAkwAlgorithm(int hashBitLength, KeyAlgorithm request) throws final Password key = Assert.notNull(request.getKey(), "Decryption Password cannot be null."); ParameterReadable reader = new RequiredParameterReader(header); final byte[] inputSalt = reader.get(DefaultJweHeader.P2S); - final int iterations = reader.get(DefaultJweHeader.P2C); + + Parameter param = DefaultJweHeader.P2C; + final int iterations = reader.get(param); + if (iterations > MAX_ITERATIONS) { + String msg = "JWE Header " + param + " value " + iterations + " exceeds " + getId() + " maximum " + + "allowed value " + MAX_ITERATIONS + ". The larger value is rejected to help mitigate " + + "potential Denial of Service attacks."; + throw new UnsupportedJwtException(msg); + } + final byte[] rfcSalt = Bytes.concat(SALT_PREFIX, inputSalt); final char[] password = key.toCharArray(); // password will be safely cleaned/zeroed in deriveKey next: final SecretKey derivedKek = deriveKey(request, password, rfcSalt, iterations); diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/security/Pbes2HsAkwAlgorithmTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/security/Pbes2HsAkwAlgorithmTest.groovy index 33513d5f8..36142646e 100644 --- a/impl/src/test/groovy/io/jsonwebtoken/impl/security/Pbes2HsAkwAlgorithmTest.groovy +++ b/impl/src/test/groovy/io/jsonwebtoken/impl/security/Pbes2HsAkwAlgorithmTest.groovy @@ -16,8 +16,11 @@ package io.jsonwebtoken.impl.security import io.jsonwebtoken.Jwts +import io.jsonwebtoken.UnsupportedJwtException import io.jsonwebtoken.impl.DefaultJweHeaderMutator import io.jsonwebtoken.impl.DefaultMutableJweHeader +import io.jsonwebtoken.io.Encoders +import io.jsonwebtoken.lang.Strings import io.jsonwebtoken.security.KeyRequest import io.jsonwebtoken.security.Keys import io.jsonwebtoken.security.Password @@ -50,6 +53,36 @@ class Pbes2HsAkwAlgorithmTest { } } + @Test + void testExceedsMaxIterations() { + for (Pbes2HsAkwAlgorithm alg : ALGS) { + def password = Keys.password('correct horse battery staple'.toCharArray()) + def iterations = alg.MAX_ITERATIONS + 1 + // we make the JWE string directly from JSON here (instead of using Jwts.builder()) to avoid + // the computational time it would take to create such JWEs with excessive iterations as well as + // avoid the builder throwing any exceptions (and this is what a potential attacker would do anyway): + def headerJson = """ + { + "p2c": ${iterations}, + "p2s": "831BG_z_ZxkN7Rnt5v1iYm1A0bn6VEuxpW4gV7YBMoE", + "alg": "${alg.id}", + "enc": "A256GCM" + }""" + def jwe = Encoders.BASE64URL.encode(Strings.utf8(headerJson)) + + '.OSAhMk3FtaCeZ5v1c8bWBgssEVqx2mCPUEnJUsg4hwIQyrUP-LCYkg.' + + 'K4R_-zb4qaZ3R0W8.sGS4mcT_xBhZC1d7G-g.kWqd_4sEsaKrWE_hMZ5HmQ' + try { + Jwts.parser().decryptWith(password).build().parse(jwe) + } catch (UnsupportedJwtException expected) { + String msg = "JWE Header 'p2c' (PBES2 Count) value ${iterations} exceeds ${alg.id} maximum allowed " + + "value ${alg.MAX_ITERATIONS}. The larger value is rejected to help mitigate potential " + + "Denial of Service attacks." + //println msg + assertEquals msg, expected.message + } + } + } + // for manual/developer testing only. Takes a long time and there is no deterministic output to assert /* @Test