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!

Author: lhazlewood

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

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<SecretKey, SecretKey> wrapAlg;
 
     private static byte[] toRfcSaltPrefix(byte[] bytes) {
@@ -106,6 +110,7 @@ protected Pbes2HsAkwAlgorithm(int hashBitLength, KeyAlgorithm<SecretKey, SecretK
         } else {
             DEFAULT_ITERATIONS = DEFAULT_SHA256_ITERATIONS;
         }
+        MAX_ITERATIONS = (int) (DEFAULT_ITERATIONS * MAX_ITERATIONS_FACTOR); // upper bound to help mitigate DoS attacks
 
         // https://www.rfc-editor.org/rfc/rfc7518.html#section-4.8, 2nd paragraph, last sentence:
         // "Their derived-key lengths respectively are 16, 24, and 32 octets." :
@@ -184,7 +189,16 @@ public SecretKey getDecryptionKey(DecryptionKeyRequest<Password> 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<Integer> 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);

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