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.

Sign up for GitHub

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

Jump to bottom

JWE arbitrary content compression #937

Merged
merged 4 commits into from
Apr 25, 2024
Merged

JWE arbitrary content compression #937

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c83fbe9
Fix compression being omitted for JWEs with arbitrary content
mnylen Apr 22, 2024
22b946f
Refactor duplicate payload -> input stream logic from sign()/encrypt()
mnylen Apr 23, 2024
9e94665
Preserve the content name
mnylen Apr 24, 2024
6d88e34
Fix name for JWE payload
mnylen Apr 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 3 additions & 4 deletions impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtBuilder.java
View file
Open in desktop
Expand Up @@ -693,14 +693,13 @@ private String encrypt(final Payload content, final Key key, final Provider keyP
Assert.stateNotNull(keyAlgFunction, "KeyAlgorithm function cannot be null.");
assertPayloadEncoding("JWE");

InputStream plaintext;
ByteArrayOutputStream out = new ByteArrayOutputStream(4096);
if (content.isClaims()) {
ByteArrayOutputStream out = new ByteArrayOutputStream(4096);
writeAndClose("JWE Claims", content, out);
plaintext = Streams.of(out.toByteArray());
} else {
plaintext = content.toInputStream();
writeAndClose("JWE Content", content, out);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to be careful here: if compression is not enabled, we don't want to write the plaintext Inputstream to an intermediate output stream (and then converted back into an input stream) which would be an unnecessary roundtrip/copy.

This logic should look more like that found in the sign method:

jjwt/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtBuilder.java

Lines 598 to 605 in c673b76

if (payload.isClaims() || payload.isCompressed()) {
ByteArrayOutputStream claimsOut = new ByteArrayOutputStream(8192);
writeAndClose("JWS Unencoded Payload", payload, claimsOut);
payloadStream = Streams.of(claimsOut.toByteArray());
} else {
// No claims and not compressed, so just get the direct InputStream:
payloadStream = Assert.stateNotNull(payload.toInputStream(), "Payload InputStream cannot be null.");
}

If possible, it'd be ideal if we can move this logic to a helper method that is shared by both sign and encrypt

Copy link
Contributor Author

@mnylen mnylen Apr 23, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense.

I wonder though if we should also be a bit more clever with the initial byte array size for ByteArrayOutputStream. Seems a bit wasteful to do 8 KB byte array allocation if the content is only few hundred bytes. At least if the payload has byte array / string, we could use the initial payload size as the starting point, as even with compression, we shouldn't (at least usually) go above that size.

Edit: Well, maybe I'll try to make a different PR about the initial sizes later. Noticed that writeAndClose also creates a 4KB buffer, when it could also just use the initial payload size as the starting point.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}
InputStream plaintext = Streams.of(out.toByteArray());

//only expose (mutable) JweHeader functionality to KeyAlgorithm instances, not the full headerBuilder
// (which exposes this JwtBuilder and shouldn't be referenced by KeyAlgorithms):
Expand Down
92 changes: 92 additions & 0 deletions impl/src/test/groovy/io/jsonwebtoken/JwtsTest.groovy
View file
Open in desktop
Expand Up @@ -22,6 +22,7 @@ import io.jsonwebtoken.impl.io.Streams
import io.jsonwebtoken.impl.lang.Bytes
import io.jsonwebtoken.impl.lang.Services
import io.jsonwebtoken.impl.security.*
import io.jsonwebtoken.io.CompressionAlgorithm
import io.jsonwebtoken.io.Decoders
import io.jsonwebtoken.io.Deserializer
import io.jsonwebtoken.io.Encoders
Expand Down Expand Up @@ -1398,6 +1399,97 @@ class JwtsTest {
}
}

@Test
void testJweCompressionWithArbitraryContentString() {
def codecs = [Jwts.ZIP.DEF, Jwts.ZIP.GZIP]

for (CompressionAlgorithm zip : codecs) {

for (AeadAlgorithm enc : Jwts.ENC.get().values()) {

SecretKey key = enc.key().build()

String payload = 'hello, world!'

// encrypt and compress:
String jwe = Jwts.builder()
.content(payload)
.compressWith(zip)
.encryptWith(key, enc)
.compact()

//decompress and decrypt:
def jwt = Jwts.parser()
.decryptWith(key)
.build()
.parseEncryptedContent(jwe)
assertEquals payload, new String(jwt.getPayload(), StandardCharsets.UTF_8)
}
}
}

@Test
void testJweCompressionWithArbitraryContentByteArray() {
def codecs = [Jwts.ZIP.DEF, Jwts.ZIP.GZIP]

for (CompressionAlgorithm zip : codecs) {

for (AeadAlgorithm enc : Jwts.ENC.get().values()) {

SecretKey key = enc.key().build()

byte[] payload = new byte[14];
Randoms.secureRandom().nextBytes(payload)

// encrypt and compress:
String jwe = Jwts.builder()
.content(payload)
.compressWith(zip)
.encryptWith(key, enc)
.compact()

//decompress and decrypt:
def jwt = Jwts.parser()
.decryptWith(key)
.build()
.parseEncryptedContent(jwe)
assertArrayEquals payload, jwt.getPayload()
}
}
}

@Test
void testJweCompressionWithArbitraryContentInputStream() {
def codecs = [Jwts.ZIP.DEF, Jwts.ZIP.GZIP]

for (CompressionAlgorithm zip : codecs) {

for (AeadAlgorithm enc : Jwts.ENC.get().values()) {

SecretKey key = enc.key().build()

byte[] payloadBytes = new byte[14];
Randoms.secureRandom().nextBytes(payloadBytes)

ByteArrayInputStream payload = new ByteArrayInputStream(payloadBytes)

// encrypt and compress:
String jwe = Jwts.builder()
.content(payload)
.compressWith(zip)
.encryptWith(key, enc)
.compact()

//decompress and decrypt:
def jwt = Jwts.parser()
.decryptWith(key)
.build()
.parseEncryptedContent(jwe)
assertArrayEquals payloadBytes, jwt.getPayload()
}
}
}

@Test
void testPasswordJwes() {

Expand Down