diff --git a/webpush/src/main/java/com/google/crypto/tink/apps/webpush/WebPushConstants.java b/webpush/src/main/java/com/google/crypto/tink/apps/webpush/WebPushConstants.java index c864021..aa7218b 100644 --- a/webpush/src/main/java/com/google/crypto/tink/apps/webpush/WebPushConstants.java +++ b/webpush/src/main/java/com/google/crypto/tink/apps/webpush/WebPushConstants.java @@ -63,6 +63,13 @@ final class WebPushConstants { static final int CIPHERTEXT_OVERHEAD = CONTENT_CODING_HEADER_SIZE + PADDING_DELIMETER_SIZE + DEFAULT_PADDING_SIZE + TAG_SIZE; + // https://www.rfc-editor.org/rfc/rfc8188#section-2.1 + // > rs: Values smaller than 18 are invalid. + // + // * min data length: 1 + // * padding delimeter: 1 + // * AES-GCM tag size: 16 + static final int MIN_RS = 18; static final int MAX_CIPHERTEXT_SIZE = 4096; static final String HMAC_SHA256 = "HMACSHA256"; diff --git a/webpush/src/main/java/com/google/crypto/tink/apps/webpush/WebPushHybridDecrypt.java b/webpush/src/main/java/com/google/crypto/tink/apps/webpush/WebPushHybridDecrypt.java index 9de4a66..fb264c5 100644 --- a/webpush/src/main/java/com/google/crypto/tink/apps/webpush/WebPushHybridDecrypt.java +++ b/webpush/src/main/java/com/google/crypto/tink/apps/webpush/WebPushHybridDecrypt.java @@ -111,7 +111,10 @@ private WebPushHybridDecrypt(Builder builder) throws GeneralSecurityException { } this.authSecret = builder.authSecret; - if (builder.recordSize < WebPushConstants.CIPHERTEXT_OVERHEAD + // Nothing prevent a max bound for rs. But we know that push messages + // are at most MAX_CIPHERTEXT_SIZE (0x1000), and it is usually set to + // this value for convenience. + if (builder.recordSize < WebPushConstants.MIN_RS || builder.recordSize > WebPushConstants.MAX_CIPHERTEXT_SIZE) { throw new IllegalArgumentException( String.format( @@ -228,9 +231,10 @@ public byte[] decrypt(final byte[] ciphertext, final byte[] contextInfo /* unuse record.get(salt); int recordSize = record.getInt(); - if (recordSize != this.recordSize - || recordSize < ciphertext.length - || recordSize > WebPushConstants.MAX_CIPHERTEXT_SIZE) { + // Fail if the recordSize is too low for the ciphertext, + // or higher than the recordSize we defined + if (recordSize > this.recordSize + || recordSize < (ciphertext.length - WebPushConstants.CONTENT_CODING_HEADER_SIZE)) { throw new GeneralSecurityException("invalid record size: " + recordSize); } diff --git a/webpush/src/main/java/com/google/crypto/tink/apps/webpush/WebPushHybridEncrypt.java b/webpush/src/main/java/com/google/crypto/tink/apps/webpush/WebPushHybridEncrypt.java index 4d1d5fb..7b7e3cd 100644 --- a/webpush/src/main/java/com/google/crypto/tink/apps/webpush/WebPushHybridEncrypt.java +++ b/webpush/src/main/java/com/google/crypto/tink/apps/webpush/WebPushHybridEncrypt.java @@ -134,7 +134,7 @@ public Builder() {} */ @CanIgnoreReturnValue public Builder withRecordSize(int val) { - if (val < WebPushConstants.CIPHERTEXT_OVERHEAD + if (val < WebPushConstants.MIN_RS || val > WebPushConstants.MAX_CIPHERTEXT_SIZE) { throw new IllegalArgumentException( String.format( @@ -216,7 +216,8 @@ public byte[] encrypt(final byte[] plaintext, final byte[] contextInfo /* unused throw new GeneralSecurityException("contextInfo must be null because it is unused"); } - if (plaintext.length > recordSize - paddingSize - WebPushConstants.CIPHERTEXT_OVERHEAD) { + // rs >= len(plaintext + AES-GCM tag + padding delimiter + padding) + if (plaintext.length > recordSize - paddingSize - WebPushConstants.TAG_SIZE - 1) { throw new GeneralSecurityException( String.format( "plaintext too long; with record size = %d and padding size = %d, plaintext cannot"