Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PBES2 decryption maximum iterations #911

Merged
merged 2 commits into from
Jan 28, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Ensured there is an upper bound (maximum) iterations enforced for PBE…
…S2 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!
lhazlewood committed Jan 28, 2024
commit f1b919a0738416c6097fad5b497b2e525fe2d8fb
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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.
* Fixes various typos in documentation and JavaDoc. Thanks to those contributing pull requests for these!

### 0.12.3
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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