From 9c520afadae9ffd8ea653114e5fdaf5698b33156 Mon Sep 17 00:00:00 2001 From: sujan kota Date: Thu, 27 Aug 2026 17:30:13 -0400 Subject: [PATCH 1/6] feat(sdk): remove 64GiB TDF limit, use counter-based payload IVs (DSPX-4495) Removes MAX_TDF_INPUT_SIZE (68719476736) and the size check in createTDF. That constant was GCM's per-invocation plaintext limit (2^39-256 bits) misapplied to the whole TDF input; each segment is its own invocation and is capped at 4MiB by Config.MAX_SEGMENT_SIZE, so the bound was never relevant. SDK.DataSizeNotSupported is retained as public API but is no longer thrown. Replaces the random per-segment AES-GCM nonce with a deterministic unsigned 96-bit big-endian counter (TDF.IvCounter). NIST SP 800-38D prefers the deterministic construction; the RBG-based one it replaces carries a birthday bound that a counter does not have. Metadata is encrypted with the per-split symKey while the payload uses the XOR of all split keys, so with a single key split the two are the same key. IV 0 is therefore reserved for the metadata and payload segments start at IV 1. The counter enforces the SP 800-38D 8.3 cap of 2^32 invocations per key and refuses to wrap, so an IV can never be issued twice. Neither limit is reachable in practice - at the 16KiB minimum segment size the cap is 64TiB of input - but the invariant now holds by construction rather than by assumption. Every segment is still prefixed with its 12-byte IV, so the wire format is unchanged and existing TDFs continue to decrypt. Signed-off-by: sujan kota --- .../java/io/opentdf/platform/sdk/SDK.java | 4 +- .../java/io/opentdf/platform/sdk/TDF.java | 136 ++++++++--- .../java/io/opentdf/platform/sdk/TDFTest.java | 220 ++++++++++++++---- 3 files changed, 285 insertions(+), 75 deletions(-) diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/SDK.java b/sdk/src/main/java/io/opentdf/platform/sdk/SDK.java index 16c7a35a..818a9b90 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/SDK.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/SDK.java @@ -390,8 +390,8 @@ public SplitKeyException(String errorMessage) { } /** - * {@link DataSizeNotSupported} is thrown when the user attempts to create - * a TDF with a size larger than the maximum size (currently 64GiB). + * Legacy exception type retained for compatibility. TDF creation no longer + * imposes a fixed input-size limit. */ public static class DataSizeNotSupported extends SDKException { public DataSizeNotSupported(String errorMessage) { diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java b/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java index 8827cb85..959d5fd7 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java @@ -55,35 +55,14 @@ private static byte[] tdfECKeySaltCompute() { */ public static final String TDF_SPEC_VERSION = "4.3.0"; private static final String KEY_ACCESS_SCHEMA_VERSION = "1.0"; - private final long maximumSize; - private final SDK.Services services; - /** - * Constructs a new TDF instance using the default maximum input size defined by - * MAX_TDF_INPUT_SIZE. - *

- * This constructor is primarily used to initialize the TDF object with the - * standard maximum - * input size, which controls the maximum size of the input data that can be - * processed. - * For test purposes, an alternative constructor allows for setting a custom - * maximum input size. - */ TDF(SDK.Services services) { - this(MAX_TDF_INPUT_SIZE, services); - } - - // constructor for tests so that we can set a maximum size that's tractable for - // tests - TDF(long maximumInputSize, SDK.Services services) { - this.maximumSize = maximumInputSize; this.services = services; } private static final Logger logger = LoggerFactory.getLogger(TDF.class); - private static final long MAX_TDF_INPUT_SIZE = 68719476736L; private static final int GCM_KEY_SIZE = 32; private static final String kSplitKeyType = "split"; private static final String kWrapped = "wrapped"; @@ -103,6 +82,100 @@ private static byte[] tdfECKeySaltCompute() { private static final Gson gson = new GsonBuilder().create(); + /** + * NIST SP 800-38D section 8.3 caps the total number of AES-GCM + * authenticated-encryption invocations under a single key at 2^32. One + * invocation is spent on the metadata (IV 0), leaving 2^32 - 1 for payload + * segments. + *

+ * This is not reachable in practice — at the smallest permitted segment size + * ({@link Config#MIN_SEGMENT_SIZE}, 16 KiB) it would take 64 TiB of input — but + * it is enforced so the invariant holds by construction rather than by + * assumption. + */ + static final long MAX_GCM_INVOCATIONS_PER_KEY = 1L << 32; + + /** + * A deterministic, unsigned 96-bit big-endian AES-GCM IV counter. + *

+ * A TDF encrypts its metadata and its payload segments under keys that are + * identical when there is a single key split, so the two must never share an + * IV. IV 0 is reserved for the metadata and payload segments start at IV 1, + * incrementing once per segment. + *

+ * The counter refuses to issue an IV once its invocation budget is spent, and + * refuses to wrap past its maximum value, so an IV can never be handed out + * twice. + *

+ * Precondition: this is safe only because the key is freshly generated + * for every TDF ({@code AesGcm.generateKey()} in {@code prepareManifest}). + * Reusing a key across two TDFs would repeat this IV sequence, which is + * catastrophic for AES-GCM — it leaks the XOR of the plaintexts and enables + * authentication-key recovery. Do not add a way to supply or reuse a payload + * key without also changing this construction. + */ + static final class IvCounter { + private final byte[] nextIv; + private long remainingInvocations; + private boolean wrapped; + + /** + * The IV reserved for encrypting the TDF metadata. + * + * @return twelve zero bytes + */ + static byte[] metadataIv() { + return new byte[kGcmIvSize]; + } + + /** + * A payload IV counter whose first value is 1, leaving IV 0 for the metadata + * and the remainder of the per-key invocation budget for payload segments. + */ + static IvCounter forPayload() { + byte[] initialIv = new byte[kGcmIvSize]; + initialIv[initialIv.length - 1] = 1; + return new IvCounter(initialIv, MAX_GCM_INVOCATIONS_PER_KEY - 1); + } + + IvCounter(byte[] initialIv, long invocationBudget) { + Objects.requireNonNull(initialIv, "initial IV"); + if (initialIv.length != kGcmIvSize) { + throw new IllegalArgumentException("invalid IV size: " + initialIv.length); + } + if (invocationBudget < 0) { + throw new IllegalArgumentException("invalid invocation budget: " + invocationBudget); + } + this.nextIv = initialIv.clone(); + this.remainingInvocations = invocationBudget; + } + + byte[] next() { + if (remainingInvocations <= 0) { + throw new SDKException("exceeded the maximum of " + MAX_GCM_INVOCATIONS_PER_KEY + + " AES-GCM invocations for a single key"); + } + if (wrapped) { + throw new SDKException("AES-GCM IV counter exhausted"); + } + + byte[] currentIv = nextIv.clone(); + remainingInvocations--; + wrapped = increment(nextIv); + return currentIv; + } + + private static boolean increment(byte[] iv) { + for (int index = iv.length - 1; index >= 0; index--) { + iv[index]++; + if (iv[index] != 0) { + return false; + } + } + return true; + } + } + static class EncryptedMetadata { private String ciphertext; private String iv; @@ -176,12 +249,17 @@ private void prepareManifest(Config.TDFConfig tdfConfig, Map(); - long totalSize = 0; boolean finished; try (var payloadOutput = tdfWriter.payload()) { do { @@ -420,18 +498,14 @@ TDFObject createTDF(InputStream payload, OutputStream outputStream, Config.TDFCo readThisLoop += nRead; } finished = nRead < 0; - totalSize += readThisLoop; - - if (totalSize > maximumSize) { - throw new SDK.DataSizeNotSupported("can't create tdf larger than 64gb"); - } byte[] cipherData; byte[] segmentSig; Manifest.Segment segmentInfo = new Manifest.Segment(); // encrypt - cipherData = tdfObject.aesGcm.encrypt(readBuf, 0, readThisLoop).asBytes(); + cipherData = tdfObject.aesGcm.encrypt(payloadIv.next(), kAesBlockSize, + readBuf, 0, readThisLoop); payloadOutput.write(cipherData); segmentSig = calculateSignature(cipherData, tdfObject.payloadKey, tdfConfig.segmentIntegrityAlgorithm); diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java index 74ef151e..b2803773 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java @@ -6,6 +6,7 @@ import com.nimbusds.jose.JOSEException; import com.nimbusds.jose.jwk.JWK; import com.google.gson.Gson; +import com.google.gson.JsonObject; import io.opentdf.platform.policy.KeyAccessServer; import io.opentdf.platform.policy.kasregistry.KeyAccessServerRegistryServiceClient; import io.opentdf.platform.policy.kasregistry.ListKeyAccessServersRequest; @@ -21,22 +22,24 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.lang.reflect.Modifier; import java.nio.charset.StandardCharsets; import java.security.KeyPair; import java.security.cert.X509Certificate; import java.util.ArrayList; +import java.util.Arrays; import java.util.Base64; import java.util.Collections; import java.util.Map; import java.util.List; import java.util.Random; -import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Predicate; import java.util.regex.Pattern; import java.util.stream.Collectors; import static io.opentdf.platform.sdk.TDF.GLOBAL_KEY_SALT; import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; @@ -697,7 +700,26 @@ public void testCreatingTDFWithMultipleSegments() throws Exception { var tdf = new TDF( new FakeServicesBuilder().setKas(kas) .setKeyAccessServerRegistryService(kasRegistryService).build()); - tdf.createTDF(plainTextInputStream, tdfOutputStream, config); + var tdfObject = tdf.createTDF(plainTextInputStream, tdfOutputStream, config); + + var segments = tdfObject.getManifest().encryptionInformation.integrityInformation.segments; + assertThat(segments.size()) + .withFailMessage("test needs more than one segment to be meaningful") + .isGreaterThan(1); + + // payload segments start at IV 1 (IV 0 is reserved for the metadata) and + // increment by one for every segment + var seenIvs = new ArrayList(); + var encryptedReader = new TDFReader(new SeekableInMemoryByteChannel(tdfOutputStream.toByteArray())); + for (int segmentIndex = 0; segmentIndex < segments.size(); segmentIndex++) { + byte[] encryptedSegment = new byte[(int) segments.get(segmentIndex).encryptedSegmentSize]; + assertThat(encryptedReader.readPayloadBytes(encryptedSegment)).isEqualTo(encryptedSegment.length); + byte[] iv = Arrays.copyOf(encryptedSegment, AesGcm.GCM_NONCE_LENGTH); + assertThat(iv).containsExactly(bigEndianIv(segmentIndex + 1)); + seenIvs.add(Base64.getEncoder().encodeToString(iv)); + } + assertThat(seenIvs).doesNotHaveDuplicates(); + var unwrappedData = new ByteArrayOutputStream(); var reader = tdf.loadTDF(new SeekableInMemoryByteChannel(tdfOutputStream.toByteArray()), platformUrl); reader.readPayload(unwrappedData); @@ -708,52 +730,166 @@ public void testCreatingTDFWithMultipleSegments() throws Exception { } - @Test - public void testCreatingTooLargeTDF() { - var random = new Random(); - var maxSize = random.nextInt(1024); - var numReturned = new AtomicInteger(0); - - // return 1 more byte than the maximum size - var is = new InputStream() { - @Override - public int read() { - if (numReturned.get() > maxSize) { - return -1; - } - numReturned.incrementAndGet(); - return 1; - } + /** + * The unsigned 96-bit big-endian encoding of {@code value}, for asserting on + * expected IVs. + */ + private static byte[] bigEndianIv(long value) { + byte[] iv = new byte[AesGcm.GCM_NONCE_LENGTH]; + for (int index = iv.length - 1; index >= 0 && value != 0; index--) { + iv[index] = (byte) value; + value >>>= 8; + } + return iv; + } - @Override - public int read(byte[] b, int off, int len) { - var numToReturn = Math.min(len, maxSize - numReturned.get() + 1); - numReturned.addAndGet(numToReturn); - return numToReturn; - } - }; + @Test + public void testMetadataUsesIvZero() throws Exception { + Config.TDFConfig config = Config.newTDFConfig( + Config.withAutoconfigure(false), + Config.withKasInformation(getRSAKASInfos()), + Config.withMetaData("here is some metadata")); - var os = new OutputStream() { - @Override - public void write(int b) { - } + var tdfOutputStream = new ByteArrayOutputStream(); + var tdf = new TDF( + new FakeServicesBuilder().setKas(kas) + .setKeyAccessServerRegistryService(kasRegistryService).build()); + var tdfObject = tdf.createTDF(new ByteArrayInputStream("some data".getBytes(StandardCharsets.UTF_8)), + tdfOutputStream, config); + + var keyAccessObjects = tdfObject.getManifest().encryptionInformation.keyAccessObj; + assertThat(keyAccessObjects).isNotEmpty(); + for (Manifest.KeyAccess keyAccess : keyAccessObjects) { + var encryptedMetadata = new Gson().fromJson( + new String(Base64.getDecoder().decode(keyAccess.encryptedMetadata), StandardCharsets.UTF_8), + JsonObject.class); + + assertThat(Base64.getDecoder().decode(encryptedMetadata.get("iv").getAsString())) + .withFailMessage("metadata IV is not zero") + .containsExactly(new byte[AesGcm.GCM_NONCE_LENGTH]); + // the ciphertext field carries the IV as a prefix as well + assertThat(Arrays.copyOf( + Base64.getDecoder().decode(encryptedMetadata.get("ciphertext").getAsString()), + AesGcm.GCM_NONCE_LENGTH)) + .containsExactly(new byte[AesGcm.GCM_NONCE_LENGTH]); + } - @Override - public void write(byte[] b, int off, int len) { - } - }; + var reader = tdf.loadTDF(new SeekableInMemoryByteChannel(tdfOutputStream.toByteArray()), platformUrl); + assertThat(reader.getMetadata()).isEqualTo("here is some metadata"); + } - var tdf = new TDF(maxSize, new FakeServicesBuilder().setKas(kas).build()); - var tdfConfig = Config.newTDFConfig( + @Test + public void testFirstPayloadSegmentUsesIvOne() throws Exception { + Config.TDFConfig config = Config.newTDFConfig( Config.withAutoconfigure(false), Config.withKasInformation(getRSAKASInfos()), - Config.withSegmentSize(Config.MIN_SEGMENT_SIZE)); - assertThrows(SDK.DataSizeNotSupported.class, - () -> tdf.createTDF(is, os, tdfConfig), - "didn't throw an exception when we created TDF that was too large"); - assertThat(numReturned.get()) - .withFailMessage("test returned the wrong number of bytes") - .isEqualTo(maxSize + 1); + Config.withMetaData("here is some metadata")); + + var tdfOutputStream = new ByteArrayOutputStream(); + var tdf = new TDF( + new FakeServicesBuilder().setKas(kas) + .setKeyAccessServerRegistryService(kasRegistryService).build()); + var tdfObject = tdf.createTDF(new ByteArrayInputStream("some data".getBytes(StandardCharsets.UTF_8)), + tdfOutputStream, config); + + var segments = tdfObject.getManifest().encryptionInformation.integrityInformation.segments; + var encryptedReader = new TDFReader(new SeekableInMemoryByteChannel(tdfOutputStream.toByteArray())); + byte[] firstSegment = new byte[(int) segments.get(0).encryptedSegmentSize]; + assertThat(encryptedReader.readPayloadBytes(firstSegment)).isEqualTo(firstSegment.length); + + assertThat(Arrays.copyOf(firstSegment, AesGcm.GCM_NONCE_LENGTH)) + .withFailMessage("first payload segment must use IV 1, leaving IV 0 for the metadata") + .containsExactly(bigEndianIv(1)); + } + + @Test + public void testPayloadIvCounterStartsAtOne() { + var counter = TDF.IvCounter.forPayload(); + + assertThat(TDF.IvCounter.metadataIv()).containsExactly(bigEndianIv(0)); + assertThat(counter.next()).containsExactly(bigEndianIv(1)); + assertThat(counter.next()).containsExactly(bigEndianIv(2)); + assertThat(counter.next()).containsExactly(bigEndianIv(3)); + } + + @Test + public void testPayloadIvCounterIncrementsWithCarry() { + // one below a two-byte carry boundary + var counter = new TDF.IvCounter(new byte[] { + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, (byte) 0xff, (byte) 0xff + }, 3); + + assertThat(counter.next()).containsExactly( + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, (byte) 0xff, (byte) 0xff); + assertThat(counter.next()).containsExactly( + 0, 1, 2, 3, 4, 5, 6, 7, 8, 10, 0, 0); + assertThat(counter.next()).containsExactly( + 0, 1, 2, 3, 4, 5, 6, 7, 8, 10, 0, 1); + } + + @Test + public void testPayloadIvCounterRejectsReuseAfterOverflow() { + byte[] finalIv = new byte[AesGcm.GCM_NONCE_LENGTH]; + Arrays.fill(finalIv, (byte) 0xff); + var counter = new TDF.IvCounter(finalIv, Long.MAX_VALUE); + + assertThat(counter.next()).containsExactly(finalIv); + // it must refuse rather than wrap around to zero, which would collide with the + // metadata IV + assertThrows(SDKException.class, counter::next); + assertThrows(SDKException.class, counter::next); + } + + @Test + public void testPayloadIvCounterStopsAtInvocationBudget() { + var counter = new TDF.IvCounter(bigEndianIv(1), 2); + + assertThat(counter.next()).containsExactly(bigEndianIv(1)); + assertThat(counter.next()).containsExactly(bigEndianIv(2)); + + var e = assertThrows(SDKException.class, counter::next); + assertThat(e).hasMessageContaining("AES-GCM invocations for a single key"); + // and it stays refused + assertThrows(SDKException.class, counter::next); + } + + @Test + public void testPayloadIvBudgetLeavesOneInvocationForMetadata() { + // NIST SP 800-38D 8.3 caps a key at 2^32 invocations; IV 0 is the metadata, so + // the payload gets 2^32 - 1 of them + assertThat(TDF.MAX_GCM_INVOCATIONS_PER_KEY).isEqualTo(4294967296L); + + var counter = TDF.IvCounter.forPayload(); + assertThat(counter.next()).containsExactly(bigEndianIv(1)); + + var budget = assertDoesNotThrow(() -> { + var field = TDF.IvCounter.class.getDeclaredField("remainingInvocations"); + field.setAccessible(true); + return (long) field.get(counter); + }); + assertThat(budget).isEqualTo(TDF.MAX_GCM_INVOCATIONS_PER_KEY - 2); + } + + @Test + public void testNoTdfInputSizeLimit() { + for (var constructor : TDF.class.getDeclaredConstructors()) { + assertThat(constructor.getParameterTypes()) + .withFailMessage("the maximum-input-size constructor should have been removed") + .doesNotContain(long.class); + } + + for (var field : TDF.class.getDeclaredFields()) { + boolean isNumericConstant = Modifier.isStatic(field.getModifiers()) + && (field.getType().equals(long.class) || field.getType().equals(int.class)); + if (!isNumericConstant) { + continue; + } + field.setAccessible(true); + long value = assertDoesNotThrow(() -> ((Number) field.get(null)).longValue()); + assertThat(value) + .withFailMessage("TDF still declares a size limit in %s", field.getName()) + .isNotIn(68719476736L /* 64 GiB */, 10485760L /* 10 MiB */); + } } @Test From f2775aa0e95ee17d1f3986487f958831592e1321 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Tue, 1 Sep 2026 15:09:21 -0400 Subject: [PATCH 2/6] fix(sdk): write zip64 offsets past 2 GiB so large TDFs are readable The manifest is appended after the payload, so in a TDF over 2 GiB its central directory offset didn't fit in a 32-bit field. ZipWriter narrowed it with an unchecked (int) cast, so createTDF reported success and wrote a file loadTDF could not read. Pre-existing, but this branch removes the 64 GiB cap that had been hiding it above 2 GiB. - ZipWriter marks byte-array entries zip64 when the offset or size exceeds MAX_NON_ZIP64_VALUE, and fails loud rather than truncating an entry that wasn't marked. The threshold is Integer.MAX_VALUE rather than the 0xFFFFFFFE the format allows: the fields are unsigned on the wire, but readers that widen them with a signed read see 2 GiB as negative. - ZipReader reads the 32- and 16-bit header fields unsigned, with the zip64 sentinels moved in lockstep, and rejects an out-of-range local header offset instead of throwing a raw IllegalArgumentException. - Both sides write and read the zip64 extra field in APPNOTE 4.5.3 order. A no-op for STORED entries, where original and compressed size are equal; correct now for a compressed entry from another writer. Archives below 2 GiB are byte-identical to before, verified by writing the same archive with the pre-change and post-change writer. Also on the crypto path this branch touches: - IvCounter is a single long bounded at construction by MAX_GCM_INVOCATIONS_PER_KEY, so no caller can configure a counter that reaches 2^96 and wraps onto the metadata IV, and next() is synchronized. - The MAX_GCM_INVOCATIONS_PER_KEY javadoc no longer mis-cites SP 800-38D section 8.3, whose 2^32 limit is scoped to RBG-based or non-96-bit IVs and does not bind here. 2^32 is kept as a conservative ceiling. - AesGcm.encrypt validates the IV and tag lengths, and reports encryption failures as SDKException("error gcm encrypt") rather than RuntimeException("error gcm decrypt"). - SDK.DataSizeNotSupported is deprecated for removal. It extends RuntimeException, so a downstream catch still compiles and never runs. Tests: zip64 round-trips through a lowered-threshold seam so the real path runs in CI in milliseconds; the disabled 7-8 GB test now appends an entry after the big stream, which is what would have caught this. The two reflection-based IV tests are replaced with behavioral ones, including the single-split case the metadata IV reservation exists to protect. --- .../java/io/opentdf/platform/sdk/AesGcm.java | 37 ++- .../java/io/opentdf/platform/sdk/SDK.java | 12 +- .../java/io/opentdf/platform/sdk/TDF.java | 112 ++++---- .../io/opentdf/platform/sdk/TDFWriter.java | 7 + .../io/opentdf/platform/sdk/ZipReader.java | 74 ++++-- .../io/opentdf/platform/sdk/ZipWriter.java | 57 ++++- .../java/io/opentdf/platform/sdk/TDFTest.java | 241 ++++++++++++++---- .../opentdf/platform/sdk/TDFWriterTest.java | 29 +++ .../opentdf/platform/sdk/ZipWriterTest.java | 173 +++++++++++-- 9 files changed, 585 insertions(+), 157 deletions(-) diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/AesGcm.java b/sdk/src/main/java/io/opentdf/platform/sdk/AesGcm.java index 77ec3655..bed1b57b 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/AesGcm.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/AesGcm.java @@ -113,6 +113,12 @@ public Encrypted encrypt(byte[] plaintext) { /** *

encrypt.

* + *

Generates a fresh random nonce from {@link SecureRandom} for every call. A + * random 96-bit nonce is only safe for a modest number of invocations under one key, so use + * this overload only with a key used once or a very small number of times. To encrypt many + * messages under a single key, use + * {@link #encrypt(byte[], int, byte[], int, int)} with a counter that never repeats.

+ * * @param plaintext the plaintext byte array to encrypt * @param offset where the input start * @param len input length @@ -151,14 +157,25 @@ public Encrypted encrypt(byte[] plaintext, int offset, int len) { /** *

encrypt.

* - * @param iv the IV vector - * @param authTagLen the length of the auth tag + * @param iv the IV vector, which must be {@value #GCM_NONCE_LENGTH} bytes and must never be + * reused under this key + * @param authTagLen the length of the auth tag, which must be {@value #GCM_TAG_LENGTH} * @param plaintext the plaintext byte array to encrypt * @param offset where the input start * @param len input length - * @return the encrypted text + * @return the encrypted text, prefixed with the IV */ public byte[] encrypt(byte[] iv, int authTagLen, byte[] plaintext, int offset, int len) { + if (iv == null || iv.length != GCM_NONCE_LENGTH) { + throw new IllegalArgumentException( + "invalid IV size for gcm encryption: " + (iv == null ? "null" : iv.length)); + } + // strict, because the read path assumes this length: Encrypted(byte[]) splits at + // GCM_NONCE_LENGTH and TDF validates segment sizes against GCM_TAG_LENGTH, so any other + // value would write a TDF this SDK cannot read + if (authTagLen != GCM_TAG_LENGTH) { + throw new IllegalArgumentException("invalid auth tag length for gcm encryption: " + authTagLen); + } try { Cipher cipher = Cipher.getInstance(CIPHER_TRANSFORM); @@ -170,10 +187,9 @@ public byte[] encrypt(byte[] iv, int authTagLen, byte[] plaintext, int offset, i System.arraycopy(iv, 0, cipherTextWithNonce, 0, iv.length); System.arraycopy(cipherText, 0, cipherTextWithNonce, iv.length, cipherText.length); return cipherTextWithNonce; - } catch (NoSuchPaddingException | NoSuchAlgorithmException | InvalidAlgorithmParameterException e) { - throw new RuntimeException("error gcm decrypt", e); - } catch (InvalidKeyException | BadPaddingException | IllegalBlockSizeException e) { - throw new RuntimeException("error gcm decrypt", e); + } catch (NoSuchPaddingException | NoSuchAlgorithmException | InvalidAlgorithmParameterException + | InvalidKeyException | BadPaddingException | IllegalBlockSizeException e) { + throw new SDKException("error gcm encrypt", e); } } @@ -189,10 +205,9 @@ public byte[] decrypt(Encrypted cipherTextWithNonce) { GCMParameterSpec spec = new GCMParameterSpec(GCM_TAG_LENGTH * 8, cipherTextWithNonce.iv); cipher.init(Cipher.DECRYPT_MODE, key, spec); return cipher.doFinal(cipherTextWithNonce.ciphertext); - } catch (NoSuchPaddingException | NoSuchAlgorithmException | InvalidAlgorithmParameterException e) { - throw new RuntimeException("error gcm decrypt", e); - } catch (InvalidKeyException | BadPaddingException | IllegalBlockSizeException e) { - throw new RuntimeException("error gcm decrypt", e); + } catch (NoSuchPaddingException | NoSuchAlgorithmException | InvalidAlgorithmParameterException + | InvalidKeyException | BadPaddingException | IllegalBlockSizeException e) { + throw new SDKException("error gcm decrypt", e); } } diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/SDK.java b/sdk/src/main/java/io/opentdf/platform/sdk/SDK.java index 818a9b90..bc207173 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/SDK.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/SDK.java @@ -390,9 +390,17 @@ public SplitKeyException(String errorMessage) { } /** - * Legacy exception type retained for compatibility. TDF creation no longer - * imposes a fixed input-size limit. + * Legacy exception type retained for compatibility. Nothing throws it any more. + *

+ * TDF creation streams its input and writes zip64 offsets, so it no longer imposes a fixed + * input-size limit. The bounds that remain are practical rather than fixed: the manifest + * holds one record per segment and is assembled in memory, and a payload key is limited to + * 2^32 AES-GCM invocations. + *

+ * Because this extends {@link SDKException}, which is unchecked, an existing + * {@code catch (DataSizeNotSupported e)} still compiles and simply never runs. */ + @Deprecated(since = "0.19.0", forRemoval = true) public static class DataSizeNotSupported extends SDKException { public DataSizeNotSupported(String errorMessage) { super(errorMessage); diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java b/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java index 959d5fd7..ae8a2624 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java @@ -71,7 +71,6 @@ private static byte[] tdfECKeySaltCompute() { private static final String kMlkemWrapped = "mlkem-wrapped"; private static final String kKasProtocol = "kas"; private static final int kGcmIvSize = 12; - private static final int kAesBlockSize = 16; private static final String kGCMCipherAlgorithm = "AES-256-GCM"; private static final int kGMACPayloadLength = 16; private static final String kGmacIntegrityAlgorithm = "GMAC"; @@ -83,15 +82,22 @@ private static byte[] tdfECKeySaltCompute() { private static final Gson gson = new GsonBuilder().create(); /** - * NIST SP 800-38D section 8.3 caps the total number of AES-GCM - * authenticated-encryption invocations under a single key at 2^32. One - * invocation is spent on the metadata (IV 0), leaving 2^32 - 1 for payload - * segments. + * A self-imposed ceiling on the number of AES-GCM authenticated-encryption + * invocations under a single payload key. One invocation is spent on the + * metadata (IV 0), leaving 2^32 - 1 for payload segments. *

- * This is not reachable in practice — at the smallest permitted segment size - * ({@link Config#MIN_SEGMENT_SIZE}, 16 KiB) it would take 64 TiB of input — but - * it is enforced so the invariant holds by construction rather than by - * assumption. + * This follows the deterministic IV construction of NIST SP 800-38D section + * 8.2.1. Because the payload key is freshly generated for each TDF and used by a + * single device, section 8.2.1 permits an empty fixed field, so the whole 96 bits + * are the invocation field and the constraint the standard actually imposes is + * 2^96. Section 8.3's limit of 2^32 does not bind here — it is scoped to + * RBG-based IVs and to deterministic IVs that are not 96 bits — but it is adopted + * anyway as a conservative ceiling. + *

+ * It is not reachable in practice: at the smallest segment size + * {@link Config#withSegmentSize} permits ({@link Config#MIN_SEGMENT_SIZE}, 16 KiB) + * it would take 64 TiB of input. It is enforced so the invariant holds by + * construction rather than by assumption. */ static final long MAX_GCM_INVOCATIONS_PER_KEY = 1L << 32; @@ -103,9 +109,10 @@ private static byte[] tdfECKeySaltCompute() { * IV. IV 0 is reserved for the metadata and payload segments start at IV 1, * incrementing once per segment. *

- * The counter refuses to issue an IV once its invocation budget is spent, and - * refuses to wrap past its maximum value, so an IV can never be handed out - * twice. + * The counter refuses to issue an IV once it reaches its limit, and no limit can + * exceed {@link #MAX_GCM_INVOCATIONS_PER_KEY}, so an IV can never be handed out + * twice and the counter can never reach a value that would collide with the + * metadata IV. *

* Precondition: this is safe only because the key is freshly generated * for every TDF ({@code AesGcm.generateKey()} in {@code prepareManifest}). @@ -115,9 +122,14 @@ private static byte[] tdfECKeySaltCompute() { * key without also changing this construction. */ static final class IvCounter { - private final byte[] nextIv; - private long remainingInvocations; - private boolean wrapped; + /** The invocation reserved for the metadata. */ + static final long METADATA_INVOCATION = 0; + /** The first invocation available to payload segments. */ + static final long FIRST_PAYLOAD_INVOCATION = METADATA_INVOCATION + 1; + + /** Exclusive; the counter stops before issuing this invocation number. */ + private final long limit; + private long next; /** * The IV reserved for encrypting the TDF metadata. @@ -125,7 +137,7 @@ static final class IvCounter { * @return twelve zero bytes */ static byte[] metadataIv() { - return new byte[kGcmIvSize]; + return ivFor(METADATA_INVOCATION); } /** @@ -133,46 +145,50 @@ static byte[] metadataIv() { * and the remainder of the per-key invocation budget for payload segments. */ static IvCounter forPayload() { - byte[] initialIv = new byte[kGcmIvSize]; - initialIv[initialIv.length - 1] = 1; - return new IvCounter(initialIv, MAX_GCM_INVOCATIONS_PER_KEY - 1); + return new IvCounter(FIRST_PAYLOAD_INVOCATION, MAX_GCM_INVOCATIONS_PER_KEY); } - IvCounter(byte[] initialIv, long invocationBudget) { - Objects.requireNonNull(initialIv, "initial IV"); - if (initialIv.length != kGcmIvSize) { - throw new IllegalArgumentException("invalid IV size: " + initialIv.length); + /** + * @param firstInvocation the first invocation number to issue + * @param limit one past the last invocation number to issue + */ + IvCounter(long firstInvocation, long limit) { + if (firstInvocation < 0) { + throw new IllegalArgumentException("invalid first invocation: " + firstInvocation); + } + if (limit < firstInvocation) { + throw new IllegalArgumentException( + "limit " + limit + " is below the first invocation " + firstInvocation); } - if (invocationBudget < 0) { - throw new IllegalArgumentException("invalid invocation budget: " + invocationBudget); + if (limit > MAX_GCM_INVOCATIONS_PER_KEY) { + throw new IllegalArgumentException("limit " + limit + " exceeds the maximum of " + + MAX_GCM_INVOCATIONS_PER_KEY + " AES-GCM invocations for a single key"); } - this.nextIv = initialIv.clone(); - this.remainingInvocations = invocationBudget; + this.next = firstInvocation; + this.limit = limit; } - byte[] next() { - if (remainingInvocations <= 0) { + /** + * @return the next IV in the sequence, which has never been returned before + */ + synchronized byte[] next() { + if (next >= limit) { throw new SDKException("exceeded the maximum of " + MAX_GCM_INVOCATIONS_PER_KEY + " AES-GCM invocations for a single key"); } - if (wrapped) { - throw new SDKException("AES-GCM IV counter exhausted"); - } - - byte[] currentIv = nextIv.clone(); - remainingInvocations--; - wrapped = increment(nextIv); - return currentIv; + return ivFor(next++); } - private static boolean increment(byte[] iv) { - for (int index = iv.length - 1; index >= 0; index--) { - iv[index]++; - if (iv[index] != 0) { - return false; - } + /** + * Encodes an invocation number as an unsigned 96-bit big-endian IV. + */ + static byte[] ivFor(long invocation) { + byte[] iv = new byte[kGcmIvSize]; + for (int index = iv.length - 1; index >= 0 && invocation != 0; index--) { + iv[index] = (byte) invocation; + invocation >>>= Byte.SIZE; } - return true; + return iv; } } @@ -255,7 +271,7 @@ private void prepareManifest(Config.TDFConfig tdfConfig, Map= zipChannel.size()) { + throw new InvalidZipException("local header offset out of range for entry [" + + fileName + "]: " + offsetToLocalHeader); + } zipChannel.position(offsetToLocalHeader); Integer signature = readInteger(); if (signature == null || signature != LOCAL_FILE_HEADER_SIGNATURE) { @@ -177,10 +201,10 @@ public InputStream getData() throws IOException { + Short.BYTES + Integer.BYTES); - long compressedSize = readInt(); - long uncompressedSize = readInt(); - int filenameLength = readShort(); - int extrafieldLength = readShort(); + long compressedSize = readUnsignedInt(); + long uncompressedSize = readUnsignedInt(); + int filenameLength = readUnsignedShort(); + int extrafieldLength = readUnsignedShort(); final long startPosition = zipChannel.position() + filenameLength + extrafieldLength; final long endPosition = startPosition + fileSize; @@ -242,15 +266,15 @@ public Entry readCentralDirectoryFileHeader() throws IOException { short lastModFileTime = readShort(); short lastModFileDate = readShort(); int crc32 = readInt(); - long compressedSize = readInt(); - long uncompressedSize = readInt(); - int fileNameLength = readShort(); - int extraFieldLength = readShort(); - short fileCommentLength = readShort(); - int diskNumberStart = readShort(); + long compressedSize = readUnsignedInt(); + long uncompressedSize = readUnsignedInt(); + int fileNameLength = readUnsignedShort(); + int extraFieldLength = readUnsignedShort(); + int fileCommentLength = readUnsignedShort(); + int diskNumberStart = readUnsignedShort(); short internalFileAttributes = readShort(); int externalFileAttributes = readInt(); - long relativeOffsetOfLocalHeader = readInt(); + long relativeOffsetOfLocalHeader = readUnsignedInt(); ByteBuffer fileName = ByteBuffer.allocate(fileNameLength); while (fileName.hasRemaining()) { @@ -262,20 +286,22 @@ public Entry readCentralDirectoryFileHeader() throws IOException { // Parse the extra field for (final long startPos = zipChannel.position(); zipChannel.position() < startPos + extraFieldLength; ) { long fieldStart = zipChannel.position(); - int headerId = readShort(); - int dataSize = readShort(); + int headerId = readUnsignedShort(); + int dataSize = readUnsignedShort(); if (headerId == ZIP64_EXTID) { - if (compressedSize == -1) { - compressedSize = readLong(); - } - if (uncompressedSize == -1) { + // APPNOTE 4.5.3 order: original size, compressed size, then local header offset + if (uncompressedSize == ZIP64_MAGICVAL) { uncompressedSize = readLong(); } - if (relativeOffsetOfLocalHeader == -1) { + if (compressedSize == ZIP64_MAGICVAL) { + compressedSize = readLong(); + } + if (relativeOffsetOfLocalHeader == ZIP64_MAGICVAL) { relativeOffsetOfLocalHeader = readLong(); } - if (diskNumberStart == ZIP64_MAGICVAL) { + // a 2-byte field, so its sentinel is 0xFFFF rather than 0xFFFFFFFF + if (diskNumberStart == ZIP64_MAGIC_SHORT) { diskNumberStart = readInt(); } } diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/ZipWriter.java b/sdk/src/main/java/io/opentdf/platform/sdk/ZipWriter.java index 71aea34c..65357573 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/ZipWriter.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/ZipWriter.java @@ -17,6 +17,19 @@ public class ZipWriter { private static final int ZIP_VERSION = 0x2D; private static final int ZIP_64_MAGIC_VAL = 0xFFFFFFFF; + + /** + * The largest offset or size we will write into a 32-bit central directory field. Entries + * that don't fit are written as ZIP64 instead. + *

+ * This is {@link Integer#MAX_VALUE} rather than the {@code 0xFFFFFFFE} the format allows. + * The fields are unsigned on the wire, but readers that widen them with a signed read see + * anything at or above 2 GiB as negative. Switching to ZIP64 at 2 GiB costs + * {@value #ZIP_64_GLOBAL_EXTENDED_INFO_EXTRA_FIELD_SIZE} bytes per affected entry and keeps + * those readers working, including versions of this SDK that predate the unsigned reads in + * {@link ZipReader}. + */ + static final long MAX_NON_ZIP64_VALUE = Integer.MAX_VALUE; private static final long ZIP_64_END_OF_CD_RECORD_SIZE = 56; private static final int ZIP_64_GLOBAL_EXTENDED_INFO_EXTRA_FIELD_SIZE = 28; @@ -28,9 +41,44 @@ public class ZipWriter { private static final int MONTH_SHIFT = 5; private final CountingOutputStream out; private final ArrayList fileInfos = new ArrayList<>(); + private final long maxNonZip64Value; public ZipWriter(OutputStream out) { + this(out, MAX_NON_ZIP64_VALUE); + } + + /** + * Test seam. Lowering the threshold drives the real ZIP64 path in an archive small enough to + * write in a unit test. + * + * @param out the stream to write the archive to + * @param maxNonZip64Value the largest offset or size to write into a 32-bit field + */ + ZipWriter(OutputStream out, long maxNonZip64Value) { + if (maxNonZip64Value < 0 || maxNonZip64Value > MAX_NON_ZIP64_VALUE) { + throw new IllegalArgumentException( + "zip64 threshold must be between 0 and " + MAX_NON_ZIP64_VALUE + ", got " + maxNonZip64Value); + } this.out = new CountingOutputStream(out); + this.maxNonZip64Value = maxNonZip64Value; + } + + private boolean needsZip64(long offset, long size) { + return offset > maxNonZip64Value || size > maxNonZip64Value; + } + + /** + * Guards against silently truncating an entry that was not marked as ZIP64. Always checked + * against {@link #MAX_NON_ZIP64_VALUE} rather than the configured threshold: lowering the + * threshold only makes more entries ZIP64, so this can only fire on a genuine + * truncation. + */ + static void checkFitsInCentralDirectory(String name, long offset, long size) { + if (offset > MAX_NON_ZIP64_VALUE || size > MAX_NON_ZIP64_VALUE) { + throw new SDKException("cannot write zip entry [" + name + "]: offset " + offset + + " and size " + size + " do not both fit in a 32-bit central directory field" + + " and the entry was not marked zip64"); + } } public OutputStream stream(String name) throws IOException { @@ -118,6 +166,10 @@ public long finish() throws IOException { } private static void writeCentralDirectoryHeader(FileInfo fileInfo, OutputStream out) throws IOException { + if (!fileInfo.isZip64) { + checkFitsInCentralDirectory(fileInfo.filename, fileInfo.offset, fileInfo.size); + } + CDFileHeader cdFileHeader = new CDFileHeader(); cdFileHeader.generalPurposeBitFlag = fileInfo.flag; cdFileHeader.lastModifiedTime = fileInfo.fileTime; @@ -179,7 +231,7 @@ private FileInfo writeByteArray(String name, byte[] data, CountingOutputStream o fileInfo.filename = name; fileInfo.fileTime = (short) fileTime; fileInfo.fileDate = (short) fileDate; - fileInfo.isZip64 = false; + fileInfo.isZip64 = needsZip64(startPosition, data.length); return fileInfo; } @@ -371,8 +423,9 @@ void write(OutputStream out) throws IOException { buffer.order(ByteOrder.LITTLE_ENDIAN); buffer.putShort(signature); buffer.putShort(size); - buffer.putLong(compressedSize); + // APPNOTE 4.5.3 order: original size, compressed size, then local header offset buffer.putLong(originalSize); + buffer.putLong(compressedSize); buffer.putLong(localFileHeaderOffset); out.write(buffer.array()); diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java index b2803773..1d0002d1 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java @@ -22,7 +22,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.lang.reflect.Modifier; import java.nio.charset.StandardCharsets; import java.security.KeyPair; import java.security.cert.X509Certificate; @@ -30,9 +29,12 @@ import java.util.Arrays; import java.util.Base64; import java.util.Collections; +import java.util.HashSet; import java.util.Map; import java.util.List; import java.util.Random; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.function.Predicate; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -747,7 +749,7 @@ private static byte[] bigEndianIv(long value) { public void testMetadataUsesIvZero() throws Exception { Config.TDFConfig config = Config.newTDFConfig( Config.withAutoconfigure(false), - Config.withKasInformation(getRSAKASInfos()), + Config.withKasInformation(getSingleRSAKASInfo()), Config.withMetaData("here is some metadata")); var tdfOutputStream = new ByteArrayOutputStream(); @@ -782,7 +784,7 @@ public void testMetadataUsesIvZero() throws Exception { public void testFirstPayloadSegmentUsesIvOne() throws Exception { Config.TDFConfig config = Config.newTDFConfig( Config.withAutoconfigure(false), - Config.withKasInformation(getRSAKASInfos()), + Config.withKasInformation(getSingleRSAKASInfo()), Config.withMetaData("here is some metadata")); var tdfOutputStream = new ByteArrayOutputStream(); @@ -814,35 +816,32 @@ public void testPayloadIvCounterStartsAtOne() { @Test public void testPayloadIvCounterIncrementsWithCarry() { - // one below a two-byte carry boundary - var counter = new TDF.IvCounter(new byte[] { - 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, (byte) 0xff, (byte) 0xff - }, 3); + // spelled out rather than built with bigEndianIv, so this doesn't just re-derive the + // encoding it is checking. one below a two-byte carry boundary: + var counter = new TDF.IvCounter(0xFFFF, 0x10002); assertThat(counter.next()).containsExactly( - 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, (byte) 0xff, (byte) 0xff); + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, (byte) 0xff, (byte) 0xff); assertThat(counter.next()).containsExactly( - 0, 1, 2, 3, 4, 5, 6, 7, 8, 10, 0, 0); + 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0); assertThat(counter.next()).containsExactly( - 0, 1, 2, 3, 4, 5, 6, 7, 8, 10, 0, 1); + 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 1); } @Test - public void testPayloadIvCounterRejectsReuseAfterOverflow() { - byte[] finalIv = new byte[AesGcm.GCM_NONCE_LENGTH]; - Arrays.fill(finalIv, (byte) 0xff); - var counter = new TDF.IvCounter(finalIv, Long.MAX_VALUE); - - assertThat(counter.next()).containsExactly(finalIv); - // it must refuse rather than wrap around to zero, which would collide with the - // metadata IV - assertThrows(SDKException.class, counter::next); + public void testPayloadIvCounterCarriesAcrossTheFourByteBoundary() { + // an implementation that kept the counter in an int would break here + var counter = new TDF.IvCounter(0xFFFFFFFFL, TDF.MAX_GCM_INVOCATIONS_PER_KEY); + + assertThat(counter.next()).containsExactly( + 0, 0, 0, 0, 0, 0, 0, 0, (byte) 0xff, (byte) 0xff, (byte) 0xff, (byte) 0xff); + // 2^32 is the limit, so the counter stops rather than issuing it assertThrows(SDKException.class, counter::next); } @Test public void testPayloadIvCounterStopsAtInvocationBudget() { - var counter = new TDF.IvCounter(bigEndianIv(1), 2); + var counter = new TDF.IvCounter(1, 3); assertThat(counter.next()).containsExactly(bigEndianIv(1)); assertThat(counter.next()).containsExactly(bigEndianIv(2)); @@ -853,45 +852,185 @@ public void testPayloadIvCounterStopsAtInvocationBudget() { assertThrows(SDKException.class, counter::next); } + @Test + public void testPayloadIvCounterRejectsALimitThatCouldCollideWithTheMetadataIv() { + // no caller can configure a counter that runs far enough to wrap back to IV 0 + assertThrows(IllegalArgumentException.class, + () -> new TDF.IvCounter(1, TDF.MAX_GCM_INVOCATIONS_PER_KEY + 1)); + assertThrows(IllegalArgumentException.class, () -> new TDF.IvCounter(1, Long.MAX_VALUE)); + assertThrows(IllegalArgumentException.class, () -> new TDF.IvCounter(-1, 10)); + assertThrows(IllegalArgumentException.class, () -> new TDF.IvCounter(10, 9)); + + assertDoesNotThrow(() -> new TDF.IvCounter(1, TDF.MAX_GCM_INVOCATIONS_PER_KEY)); + } + @Test public void testPayloadIvBudgetLeavesOneInvocationForMetadata() { - // NIST SP 800-38D 8.3 caps a key at 2^32 invocations; IV 0 is the metadata, so - // the payload gets 2^32 - 1 of them assertThat(TDF.MAX_GCM_INVOCATIONS_PER_KEY).isEqualTo(4294967296L); - var counter = TDF.IvCounter.forPayload(); - assertThat(counter.next()).containsExactly(bigEndianIv(1)); + // the payload never issues the metadata IV + assertThat(TDF.IvCounter.forPayload().next()) + .isNotEqualTo(TDF.IvCounter.metadataIv()); - var budget = assertDoesNotThrow(() -> { - var field = TDF.IvCounter.class.getDeclaredField("remainingInvocations"); - field.setAccessible(true); - return (long) field.get(counter); - }); - assertThat(budget).isEqualTo(TDF.MAX_GCM_INVOCATIONS_PER_KEY - 2); + // and the payload's budget is exactly one short of the per-key maximum, checked at the + // boundary rather than by reading the counter's internals + var counter = new TDF.IvCounter( + TDF.MAX_GCM_INVOCATIONS_PER_KEY - 2, TDF.MAX_GCM_INVOCATIONS_PER_KEY); + assertThat(counter.next()).containsExactly(bigEndianIv(TDF.MAX_GCM_INVOCATIONS_PER_KEY - 2)); + assertThat(counter.next()).containsExactly(bigEndianIv(TDF.MAX_GCM_INVOCATIONS_PER_KEY - 1)); + assertThrows(SDKException.class, counter::next); } @Test - public void testNoTdfInputSizeLimit() { - for (var constructor : TDF.class.getDeclaredConstructors()) { - assertThat(constructor.getParameterTypes()) - .withFailMessage("the maximum-input-size constructor should have been removed") - .doesNotContain(long.class); - } + public void testPayloadIvCounterHandsOutDistinctIvsAcrossThreads() throws Exception { + int threads = 8; + int perThread = 500; + var counter = new TDF.IvCounter(1, 1 + (long) threads * perThread); - for (var field : TDF.class.getDeclaredFields()) { - boolean isNumericConstant = Modifier.isStatic(field.getModifiers()) - && (field.getType().equals(long.class) || field.getType().equals(int.class)); - if (!isNumericConstant) { - continue; + var pool = Executors.newFixedThreadPool(threads); + try { + var futures = new ArrayList>>(); + for (int t = 0; t < threads; t++) { + futures.add(pool.submit(() -> { + var mine = new ArrayList(); + for (int i = 0; i < perThread; i++) { + mine.add(Base64.getEncoder().encodeToString(counter.next())); + } + return mine; + })); + } + + var all = new ArrayList(); + for (var future : futures) { + all.addAll(future.get()); } - field.setAccessible(true); - long value = assertDoesNotThrow(() -> ((Number) field.get(null)).longValue()); - assertThat(value) - .withFailMessage("TDF still declares a size limit in %s", field.getName()) - .isNotIn(68719476736L /* 64 GiB */, 10485760L /* 10 MiB */); + assertThat(all).hasSize(threads * perThread); + assertThat(new HashSet<>(all)) + .withFailMessage("the counter handed out the same IV twice") + .hasSize(threads * perThread); + } finally { + pool.shutdownNow(); } } + @Test + public void testCreateTDFAcceptsInputSpanningManySegments() throws Exception { + // a partial trailing segment, so the expected count isn't confused by the empty segment + // createTDF's do/while emits when the input is an exact multiple of the segment size + int fullSegments = 512; + int expectedSegments = fullSegments + 1; + var data = new byte[fullSegments * Config.MIN_SEGMENT_SIZE + 100]; + new Random(31).nextBytes(data); + + var tdf = new TDF(new FakeServicesBuilder().setKas(kas) + .setKeyAccessServerRegistryService(kasRegistryService).build()); + var tdfOutputStream = new ByteArrayOutputStream(); + var tdfObject = tdf.createTDF(new ByteArrayInputStream(data), tdfOutputStream, + Config.newTDFConfig( + Config.withAutoconfigure(false), + Config.withKasInformation(getSingleRSAKASInfo()), + Config.withSegmentSize(Config.MIN_SEGMENT_SIZE))); + + assertThat(tdfObject.getManifest().encryptionInformation.integrityInformation.segments) + .hasSize(expectedSegments); + + var reader = tdf.loadTDF(new SeekableInMemoryByteChannel(tdfOutputStream.toByteArray()), + Config.newTDFReaderConfig(), platformUrl); + var decrypted = new ByteArrayOutputStream(); + reader.readPayload(decrypted); + assertThat(decrypted.toByteArray()) + .withFailMessage("a multi-segment TDF did not round trip") + .containsExactly(data); + } + + /** + * With a single key split the metadata key and the payload key are the same key, so an IV + * shared between them would be catastrophic. This is the case the IV reservation exists for. + */ + @Test + public void testSingleSplitMetadataAndPayloadNeverShareAnIv() throws Exception { + int fullSegments = 4; + int expectedSegments = fullSegments + 1; // plus a partial trailing segment + var data = new byte[fullSegments * Config.MIN_SEGMENT_SIZE + 100]; + new Random(17).nextBytes(data); + + var tdf = new TDF(new FakeServicesBuilder().setKas(kas) + .setKeyAccessServerRegistryService(kasRegistryService).build()); + var tdfOutputStream = new ByteArrayOutputStream(); + var tdfObject = tdf.createTDF(new ByteArrayInputStream(data), tdfOutputStream, + Config.newTDFConfig( + Config.withAutoconfigure(false), + Config.withKasInformation(getSingleRSAKASInfo()), + Config.withSegmentSize(Config.MIN_SEGMENT_SIZE), + Config.withMetaData("here is some metadata"))); + + var keyAccessObjects = tdfObject.getManifest().encryptionInformation.keyAccessObj; + assertThat(keyAccessObjects) + .withFailMessage("this test is only meaningful with a single key split") + .hasSize(1); + + var seen = new HashSet(); + + var encryptedMetadata = new Gson().fromJson(new String( + Base64.getDecoder().decode(keyAccessObjects.get(0).encryptedMetadata), + StandardCharsets.UTF_8), JsonObject.class); + var metadataIv = Base64.getDecoder().decode(encryptedMetadata.get("iv").getAsString()); + assertThat(metadataIv).containsExactly(bigEndianIv(0)); + seen.add(Base64.getEncoder().encodeToString(metadataIv)); + + var encryptedReader = new TDFReader(new SeekableInMemoryByteChannel(tdfOutputStream.toByteArray())); + var manifestSegments = tdfObject.getManifest().encryptionInformation.integrityInformation.segments; + assertThat(manifestSegments).hasSize(expectedSegments); + for (int i = 0; i < manifestSegments.size(); i++) { + var segment = new byte[(int) manifestSegments.get(i).encryptedSegmentSize]; + assertThat(encryptedReader.readPayloadBytes(segment)).isEqualTo(segment.length); + + var iv = Arrays.copyOf(segment, AesGcm.GCM_NONCE_LENGTH); + assertThat(iv) + .withFailMessage("payload segment %s should use IV %s", i, i + 1) + .containsExactly(bigEndianIv(i + 1)); + assertThat(seen.add(Base64.getEncoder().encodeToString(iv))) + .withFailMessage("IV reused between the metadata and payload segment %s", i) + .isTrue(); + } + } + + /** + * The deterministic IV sequence is only safe because the payload key is fresh for every TDF. + * This pins both halves: the IVs repeat, and the ciphertext does not. + */ + @Test + public void testEachTdfUsesAFreshPayloadKey() throws Exception { + var data = "the same plaintext, encrypted twice".getBytes(StandardCharsets.UTF_8); + var tdf = new TDF(new FakeServicesBuilder().setKas(kas) + .setKeyAccessServerRegistryService(kasRegistryService).build()); + + var firstSegments = new ArrayList(); + for (int run = 0; run < 2; run++) { + // a fresh config per run: createTDF and loadTDF both mutate the config they are given + var tdfOutputStream = new ByteArrayOutputStream(); + var tdfObject = tdf.createTDF(new ByteArrayInputStream(data), tdfOutputStream, + Config.newTDFConfig( + Config.withAutoconfigure(false), + Config.withKasInformation(getSingleRSAKASInfo()))); + + var segments = tdfObject.getManifest().encryptionInformation.integrityInformation.segments; + var encryptedReader = new TDFReader( + new SeekableInMemoryByteChannel(tdfOutputStream.toByteArray())); + var segment = new byte[(int) segments.get(0).encryptedSegmentSize]; + assertThat(encryptedReader.readPayloadBytes(segment)).isEqualTo(segment.length); + firstSegments.add(segment); + } + + assertThat(Arrays.copyOf(firstSegments.get(0), AesGcm.GCM_NONCE_LENGTH)) + .withFailMessage("the IV sequence is deterministic, so it should repeat") + .containsExactly(Arrays.copyOf(firstSegments.get(1), AesGcm.GCM_NONCE_LENGTH)); + + assertThat(firstSegments.get(0)) + .withFailMessage("identical ciphertext under a repeated IV means the payload key was reused") + .isNotEqualTo(firstSegments.get(1)); + } + @Test public void testCreateTDFWithMimeType() throws Exception { final String mimeType = "application/pdf"; @@ -1215,6 +1354,16 @@ private static Config.KASInfo[] getECKASInfos() { return getKASInfos(i -> i % 2 != 0); } + /** + * Exactly one KAS, so the TDF gets a single key split and the payload key is the metadata + * key. Deterministic even though {@code keypairs} is randomly sized: index 0 always exists + * and is always RSA. + */ + @Nonnull + private static Config.KASInfo[] getSingleRSAKASInfo() { + return getKASInfos(i -> i == 0); + } + private static boolean isHexChar(byte b) { return (b >= 'a' && b <= 'f') || (b >= '0' && b <= '9'); } diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/TDFWriterTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/TDFWriterTest.java index ad446bdb..2f5d22fc 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/TDFWriterTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/TDFWriterTest.java @@ -1,5 +1,6 @@ package io.opentdf.platform.sdk; +import org.apache.commons.compress.utils.SeekableInMemoryByteChannel; import org.junit.jupiter.api.Test; import java.io.ByteArrayInputStream; @@ -72,4 +73,32 @@ void simpleTDFCreate() throws IOException { writer.finish(); fileOutStream.close(); } + + /** + * The manifest is appended after the payload, so in a large TDF its local header offset + * doesn't fit in a 32-bit central directory field. Uses the lowered zip64 threshold to run + * that path against a small file. + */ + @Test + void readsBackAManifestWrittenPastTheZip64Boundary() throws IOException { + var manifest = "{\"payload\":{\"url\":\"0.payload\"}}"; + var payload = "a payload long enough to push the manifest past the threshold"; + + var out = new ByteArrayOutputStream(); + var writer = new TDFWriter(out, 8); + try (var p = writer.payload()) { + new ByteArrayInputStream(payload.getBytes(StandardCharsets.UTF_8)).transferTo(p); + } + writer.appendManifest(manifest); + writer.finish(); + + try (var chan = new SeekableInMemoryByteChannel(out.toByteArray())) { + var reader = new TDFReader(chan); + assertEquals(manifest, reader.manifest()); + + var payloadBytes = new byte[payload.length()]; + assertEquals(payload.length(), reader.readPayloadBytes(payloadBytes)); + assertEquals(payload, new String(payloadBytes, StandardCharsets.UTF_8)); + } + } } diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/ZipWriterTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/ZipWriterTest.java index 27ed1603..d1d287b2 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/ZipWriterTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/ZipWriterTest.java @@ -1,7 +1,9 @@ package io.opentdf.platform.sdk; import org.apache.commons.compress.archivers.zip.ZipArchiveEntry; +import org.apache.commons.compress.archivers.zip.ZipExtraField; import org.apache.commons.compress.archivers.zip.ZipFile; +import org.apache.commons.compress.archivers.zip.ZipShort; import org.apache.commons.compress.utils.SeekableInMemoryByteChannel; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; @@ -20,6 +22,8 @@ import java.util.Random; import java.util.zip.CRC32; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.AssertionsForClassTypes.assertThat; public class ZipWriterTest { @@ -70,11 +74,93 @@ public void createsNonZip64Archive() throws IOException { var entry2 = z.getEntry("file2.txt"); assertThat(entry1).isNotNull(); assertThat(getDataStream(z, entry2).toString(StandardCharsets.UTF_8)).isEqualTo("Here are some more things to look at"); + + assertThat(containsZip64EndOfCentralDirectory(out.toByteArray())) + .withFailMessage("expected a small byte-array-only archive to stay non-zip64") + .isFalse(); + } + + /** + * The manifest is written after the payload, so in a large TDF its local header offset is + * past the 32-bit central directory field. Uses the lowered threshold so the real zip64 path + * runs against an archive small enough to check here. + */ + @Test + public void writesReadableZip64EntriesForOffsetsPastTheThreshold() throws IOException { + var manifest = "{\"payload\":{\"protocol\":\"zip\"}}"; + var payload = "a payload long enough to push the manifest past the threshold"; + + ByteArrayOutputStream out = new ByteArrayOutputStream(); + var writer = new ZipWriter(out, 8); + writer.data("small.txt", "tiny".getBytes(StandardCharsets.UTF_8)); + try (var entry = writer.stream("0.payload")) { + new ByteArrayInputStream(payload.getBytes(StandardCharsets.UTF_8)).transferTo(entry); + } + writer.data("0.manifest.json", manifest.getBytes(StandardCharsets.UTF_8)); + writer.finish(); + + var archive = out.toByteArray(); + assertThat(containsZip64EndOfCentralDirectory(archive)) + .withFailMessage("expected the lowered threshold to produce a zip64 archive") + .isTrue(); + + // our own reader + try (var chan = new SeekableInMemoryByteChannel(archive)) { + var reader = new ZipReader(chan); + assertThat(reader.getEntries().size()).isEqualTo(3); + assertThat(readEntry(reader, "small.txt")).isEqualTo("tiny"); + assertThat(readEntry(reader, "0.payload")).isEqualTo(payload); + assertThat(readEntry(reader, "0.manifest.json")).isEqualTo(manifest); + } + + // and an independent implementation, so we aren't just agreeing with ourselves + try (var chan = new SeekableInMemoryByteChannel(archive)) { + ZipFile z = new ZipFile.Builder().setSeekableByteChannel(chan).get(); + assertThat(getDataStream(z, z.getEntry("small.txt")).toString(StandardCharsets.UTF_8)) + .isEqualTo("tiny"); + assertThat(getDataStream(z, z.getEntry("0.payload")).toString(StandardCharsets.UTF_8)) + .isEqualTo(payload); + assertThat(getDataStream(z, z.getEntry("0.manifest.json")).toString(StandardCharsets.UTF_8)) + .isEqualTo(manifest); + + // the manifest sits past the threshold, so it has to carry a zip64 extra field + // rather than a truncated 32-bit offset. without this the test would still pass + // against a writer that never marks byte array entries as zip64 + assertThat(zip64ExtraField(z, "0.manifest.json")) + .withFailMessage("the entry past the threshold was not written as zip64") + .isNotNull(); + // and an entry below the threshold is left alone + assertThat(zip64ExtraField(z, "small.txt")) + .withFailMessage("an entry below the threshold should not be zip64") + .isNull(); + } + } + + @Test + public void refusesToTruncateAnOffsetIntoThirtyTwoBits() { + assertThatThrownBy(() -> ZipWriter.checkFitsInCentralDirectory("0.manifest.json", 1L << 31, 10)) + .isInstanceOf(SDKException.class) + .hasMessageContaining("0.manifest.json"); + + assertThatThrownBy(() -> ZipWriter.checkFitsInCentralDirectory("big.bin", 0, 1L << 32)) + .isInstanceOf(SDKException.class); + + assertThatCode(() -> ZipWriter.checkFitsInCentralDirectory( + "boundary.bin", Integer.MAX_VALUE, Integer.MAX_VALUE)) + .doesNotThrowAnyException(); + } + + @Test + public void rejectsAnOutOfRangeZip64Threshold() { + var out = new ByteArrayOutputStream(); + assertThatThrownBy(() -> new ZipWriter(out, -1)).isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> new ZipWriter(out, 1L << 32)).isInstanceOf(IllegalArgumentException.class); } @Test @Disabled("this takes a long time and shouldn't run on build machines") public void testWritingLargeFile() throws IOException { + var trailingEntry = "{\"written\":\"after the big payload\"}"; var random = new Random(); // create a file between 7 and 8 GB long fileSize = 7 * (1L << 30) + (long)Math.floor(random.nextDouble() * (1L << 30)); @@ -103,10 +189,20 @@ public void testWritingLargeFile() throws IOException { try (var entry = writer.stream("a big one")) { in.transferTo(entry); } + // a byte array entry after the big stream, the way a TDF appends its manifest. + // its local header offset is past 32 bits, so it has to be written as zip64 + writer.data("0.manifest.json", trailingEntry.getBytes(StandardCharsets.UTF_8)); writer.finish(); } } + try (var chan = FileChannel.open(zipFile.toPath(), StandardOpenOption.READ)) { + var reader = new ZipReader(chan); + assertThat(readEntry(reader, "0.manifest.json")) + .withFailMessage("couldn't read back an entry written past the 32-bit offset limit") + .isEqualTo(trailingEntry); + } + var unzippedData = File.createTempFile("big-file-unzipped", ""); unzippedData.deleteOnExit(); try (var unzippedStream = new FileOutputStream(unzippedData)) { @@ -122,44 +218,33 @@ public void testWritingLargeFile() throws IOException { .isEqualTo(testFile.length()); - var buf = new byte[2048]; - var unzippedCRC = new CRC32(); - try (var inputStream = new FileInputStream(unzippedData)) { - var read = inputStream.read(buf); - unzippedCRC.update(buf, 0, read); - } + var unzippedCRC = crcOfWholeFile(unzippedData); unzippedData.delete(); - var testFileCRC = new CRC32(); - try (var inputStream = new FileInputStream(testFile)) { - var read = inputStream.read(buf); - testFileCRC.update(buf, 0, read); - } - testFile.delete(); + var testFileCRC = crcOfWholeFile(testFile); - assertThat(unzippedCRC.getValue()) + assertThat(unzippedCRC) .withFailMessage("the extracted file's CRC differs from the CRC of the test data") - .isEqualTo(testFileCRC.getValue()); + .isEqualTo(testFileCRC); var ourUnzippedData = File.createTempFile("big-file-we-unzipped", ""); ourUnzippedData.deleteOnExit(); try (var unzippedStream = new FileOutputStream(ourUnzippedData)) { try (var chan = FileChannel.open(zipFile.toPath(), StandardOpenOption.READ)) { ZipReader reader = new ZipReader(chan); - assertThat(reader.getEntries().size()).isEqualTo(1); - reader.getEntries().get(0).getData().transferTo(unzippedStream); + assertThat(reader.getEntries().size()).isEqualTo(2); + var bigEntry = reader.getEntries().stream() + .filter(e -> e.getName().equals("a big one")) + .findFirst() + .orElseThrow(); + bigEntry.getData().transferTo(unzippedStream); } } + testFile.delete(); - var ourTestFileCRC = new CRC32(); - try (var inputStream = new FileInputStream(ourUnzippedData)) { - var read = inputStream.read(buf); - ourTestFileCRC.update(buf, 0, read); - } - - assertThat(ourTestFileCRC.getValue()) + assertThat(crcOfWholeFile(ourUnzippedData)) .withFailMessage("the file we extracted differs from the CRC of the test data") - .isEqualTo(testFileCRC.getValue()); + .isEqualTo(testFileCRC); } @Nonnull @@ -168,4 +253,44 @@ private static ByteArrayOutputStream getDataStream(ZipFile z, ZipArchiveEntry en z.getInputStream(entry).transferTo(entry1Data); return entry1Data; } + + /** commons-compress keeps {@code Zip64ExtendedInformationExtraField.HEADER_ID} package-private. */ + private static final ZipShort ZIP64_HEADER_ID = new ZipShort(0x0001); + + private static ZipExtraField zip64ExtraField(ZipFile z, String name) { + return z.getEntry(name).getExtraField(ZIP64_HEADER_ID); + } + + private static String readEntry(ZipReader reader, String name) throws IOException { + var entry = reader.getEntries().stream() + .filter(e -> e.getName().equals(name)) + .findFirst() + .orElseThrow(() -> new AssertionError("no entry named " + name)); + var data = new ByteArrayOutputStream(); + entry.getData().transferTo(data); + return data.toString(StandardCharsets.UTF_8); + } + + /** Looks for the zip64 end of central directory signature, 0x06064b50 little-endian. */ + private static boolean containsZip64EndOfCentralDirectory(byte[] archive) { + for (int i = 0; i + 4 <= archive.length; i++) { + if (archive[i] == 0x50 && archive[i + 1] == 0x4b + && archive[i + 2] == 0x06 && archive[i + 3] == 0x06) { + return true; + } + } + return false; + } + + private static long crcOfWholeFile(File file) throws IOException { + var crc = new CRC32(); + var buf = new byte[1 << 16]; + try (var inputStream = new FileInputStream(file)) { + int read; + while ((read = inputStream.read(buf)) > 0) { + crc.update(buf, 0, read); + } + } + return crc.getValue(); + } } From 409915af2b2a18cf4d86f4657c5250a987420656 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Tue, 1 Sep 2026 16:41:48 -0400 Subject: [PATCH 3/6] fix(sdk): reserve IV 0 for metadata and detect zip64 from the entry-count sentinel The package-private IvCounter constructor accepted firstInvocation 0, the invocation reserved for the metadata. With a single key split the metadata and payload keys are the same key, so a counter started there would reuse an AES-GCM IV. Require FIRST_PAYLOAD_INVOCATION instead. ZipReader decided an archive was zip64 by looking only at the central directory offset sentinel. An archive with more than 65,535 entries needs zip64 for its entry count alone while its central directory still starts below 4 GiB; on such an archive the reader took the non-zip64 path, believed there were 65,535 entries, and walked off the end of the central directory. Check every sentinel-bearing field. Signed-off-by: Dave Mihalcik --- .../java/io/opentdf/platform/sdk/TDF.java | 8 +- .../io/opentdf/platform/sdk/ZipReader.java | 8 +- .../java/io/opentdf/platform/sdk/TDFTest.java | 2 + .../opentdf/platform/sdk/ZipReaderTest.java | 99 +++++++++++++++++++ 4 files changed, 113 insertions(+), 4 deletions(-) diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java b/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java index ae8a2624..54e14de1 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java @@ -149,12 +149,14 @@ static IvCounter forPayload() { } /** - * @param firstInvocation the first invocation number to issue + * @param firstInvocation the first invocation number to issue, at least + * {@link #FIRST_PAYLOAD_INVOCATION} * @param limit one past the last invocation number to issue */ IvCounter(long firstInvocation, long limit) { - if (firstInvocation < 0) { - throw new IllegalArgumentException("invalid first invocation: " + firstInvocation); + if (firstInvocation < FIRST_PAYLOAD_INVOCATION) { + throw new IllegalArgumentException("invalid first invocation: " + firstInvocation + + "; invocation " + METADATA_INVOCATION + " is reserved for the metadata"); } if (limit < firstInvocation) { throw new IllegalArgumentException( diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java b/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java index c7e6a08b..2366f2b8 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java @@ -130,7 +130,13 @@ CentralDirectoryRecord readEndOfCentralDirectory() throws IOException { long offsetToStartOfCentralDirectory = readUnsignedInt(); int commentLength = readUnsignedShort(); - if (offsetToStartOfCentralDirectory != ZIP64_MAGICVAL) { + // any one of these fields may carry the sentinel that sends its real value to the zip64 + // end of central directory record; an archive can need zip64 for its entry count alone + // while its central directory still starts below 4 GiB. the size is checked for the same + // reason even though nothing here reads it yet + if (totalNumEntries != ZIP64_MAGIC_SHORT + && sizeOfCentralDirectory != ZIP64_MAGICVAL + && offsetToStartOfCentralDirectory != ZIP64_MAGICVAL) { return new CentralDirectoryRecord(totalNumEntries, offsetToStartOfCentralDirectory); } diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java index 1d0002d1..e9760e28 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java @@ -860,6 +860,8 @@ public void testPayloadIvCounterRejectsALimitThatCouldCollideWithTheMetadataIv() assertThrows(IllegalArgumentException.class, () -> new TDF.IvCounter(1, Long.MAX_VALUE)); assertThrows(IllegalArgumentException.class, () -> new TDF.IvCounter(-1, 10)); assertThrows(IllegalArgumentException.class, () -> new TDF.IvCounter(10, 9)); + // and invocation 0 belongs to the metadata, so no payload counter can start there + assertThrows(IllegalArgumentException.class, () -> new TDF.IvCounter(0, 10)); assertDoesNotThrow(() -> new TDF.IvCounter(1, TDF.MAX_GCM_INVOCATIONS_PER_KEY)); } diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java index 087743db..2e38779a 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java @@ -4,6 +4,7 @@ import org.apache.commons.compress.archivers.zip.Zip64Mode; import org.apache.commons.compress.archivers.zip.ZipArchiveEntry; import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream; +import org.apache.commons.compress.archivers.zip.ZipFile; import org.apache.commons.compress.utils.SeekableInMemoryByteChannel; import org.junit.jupiter.api.Test; @@ -11,6 +12,8 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.RandomAccessFile; +import java.nio.ByteBuffer; +import java.nio.ByteOrder; import java.nio.channels.SeekableByteChannel; import java.nio.charset.StandardCharsets; import java.util.HashMap; @@ -149,4 +152,100 @@ public void testReadingAndWritingRandomFiles() throws IOException { assertThat(reader.getEntries().size()).isEqualTo(namesToData.size()); } + + private static final String PAYLOAD = "a payload long enough to push the manifest along"; + private static final String MANIFEST = "{\"payload\":{\"protocol\":\"zip\"}}"; + + /** Sizes of the three trailing records, which are fixed when the archive has no comment. */ + private static final int EOCD_SIZE = 22; + private static final int ZIP64_EOCD_LOCATOR_SIZE = 20; + private static final int ZIP64_EOCD_SIZE = 56; + + private static final int EOCD_SIGNATURE = 0x06054b50; + private static final int ZIP64_EOCD_SIGNATURE = 0x06064b50; + + /** + * An archive doesn't have to use the central directory offset sentinel to be zip64: one with + * more than 65,535 entries needs zip64 for its entry count alone, while its central directory + * still starts below 4 GiB. A reader that looks only at the offset takes the non-zip64 path, + * believes there are 65,535 entries, and walks off the end of the central directory. + */ + @Test + public void testReadingAZip64ArchiveThatOnlyFlagsItsEntryCount() throws IOException { + assertReadsEveryEntry(keepOnlyTheEntryCountSentinel(zip64Archive())); + } + + /** + * A zip64 archive small enough to check here. The lowered writer threshold marks the later + * entries zip64, so the writer emits a zip64 end of central directory record and fills every + * end of central directory field with its sentinel. + */ + private static byte[] zip64Archive() throws IOException { + var out = new ByteArrayOutputStream(); + var writer = new ZipWriter(out, 8); + writer.data("small.txt", "tiny".getBytes(StandardCharsets.UTF_8)); + try (var entry = writer.stream("0.payload")) { + new ByteArrayInputStream(PAYLOAD.getBytes(StandardCharsets.UTF_8)).transferTo(entry); + } + writer.data("0.manifest.json", MANIFEST.getBytes(StandardCharsets.UTF_8)); + writer.finish(); + return out.toByteArray(); + } + + /** + * Rewrites the trailing end of central directory record so the entry count is the only field + * left holding a sentinel, taking the true size and offset out of the zip64 record that + * already carries them. The result is still a valid zip64 archive; it just no longer + * announces itself through the offset, which is the field the reader used to follow. + */ + private static byte[] keepOnlyTheEntryCountSentinel(byte[] archive) { + var buf = ByteBuffer.wrap(archive).order(ByteOrder.LITTLE_ENDIAN); + + int zip64Eocd = archive.length - (EOCD_SIZE + ZIP64_EOCD_LOCATOR_SIZE + ZIP64_EOCD_SIZE); + assertThat(buf.getInt(zip64Eocd)).isEqualTo(ZIP64_EOCD_SIGNATURE); + long centralDirectorySize = buf.getLong(zip64Eocd + 40); + long centralDirectoryOffset = buf.getLong(zip64Eocd + 48); + + int eocd = archive.length - EOCD_SIZE; + assertThat(buf.getInt(eocd)).isEqualTo(EOCD_SIGNATURE); + buf.putInt(eocd + 12, (int) centralDirectorySize); + buf.putInt(eocd + 16, (int) centralDirectoryOffset); + + return archive; + } + + private static void assertReadsEveryEntry(byte[] archive) throws IOException { + try (var channel = new SeekableInMemoryByteChannel(archive)) { + var reader = new ZipReader(channel); + assertThat(reader.getEntries().size()).isEqualTo(3); + assertThat(readEntry(reader, "small.txt")).isEqualTo("tiny"); + assertThat(readEntry(reader, "0.payload")).isEqualTo(PAYLOAD); + assertThat(readEntry(reader, "0.manifest.json")).isEqualTo(MANIFEST); + } + + // and an independent implementation, so this shows the patched archive is well formed + // rather than just something our own reader happens to tolerate + try (var channel = new SeekableInMemoryByteChannel(archive)) { + var zip = new ZipFile.Builder().setSeekableByteChannel(channel).get(); + assertThat(readEntry(zip, "small.txt")).isEqualTo("tiny"); + assertThat(readEntry(zip, "0.payload")).isEqualTo(PAYLOAD); + assertThat(readEntry(zip, "0.manifest.json")).isEqualTo(MANIFEST); + } + } + + private static String readEntry(ZipReader reader, String name) throws IOException { + var entry = reader.getEntries().stream() + .filter(e -> e.getName().equals(name)) + .findFirst() + .orElseThrow(() -> new AssertionError("no entry named " + name)); + var data = new ByteArrayOutputStream(); + entry.getData().transferTo(data); + return data.toString(StandardCharsets.UTF_8); + } + + private static String readEntry(ZipFile zip, String name) throws IOException { + var data = new ByteArrayOutputStream(); + zip.getInputStream(zip.getEntry(name)).transferTo(data); + return data.toString(StandardCharsets.UTF_8); + } } \ No newline at end of file From 5a57ce083e63e0904588a91b3fedc9fe4e816c7a Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Tue, 1 Sep 2026 17:04:30 -0400 Subject: [PATCH 4/6] fix(sdk): clear the sonarcloud findings this PR introduced The segment size addition ran in int arithmetic before widening to long (java:S2184), which is the one reliability finding failing the quality gate. Also adds the missing @deprecated tag on DataSizeNotSupported (java:S1123), drops an import left unused by the test changes (java:S1128), and uses the AssertJ size assertions (java:S5838). Signed-off-by: Dave Mihalcik --- sdk/src/main/java/io/opentdf/platform/sdk/SDK.java | 2 ++ sdk/src/main/java/io/opentdf/platform/sdk/TDF.java | 2 +- sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java | 5 ++--- sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/SDK.java b/sdk/src/main/java/io/opentdf/platform/sdk/SDK.java index bc207173..5e903498 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/SDK.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/SDK.java @@ -399,6 +399,8 @@ public SplitKeyException(String errorMessage) { *

* Because this extends {@link SDKException}, which is unchecked, an existing * {@code catch (DataSizeNotSupported e)} still compiles and simply never runs. + * + * @deprecated nothing throws this any more; remove the catch block rather than replacing it. */ @Deprecated(since = "0.19.0", forRemoval = true) public static class DataSizeNotSupported extends SDKException { diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java b/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java index 54e14de1..b54901b9 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java @@ -498,7 +498,7 @@ TDFObject createTDF(InputStream payload, OutputStream outputStream, Config.TDFCo TDFObject tdfObject = new TDFObject(); tdfObject.prepareManifest(tdfConfig, splits); - long encryptedSegmentSize = tdfConfig.defaultSegmentSize + kGcmIvSize + AesGcm.GCM_TAG_LENGTH; + long encryptedSegmentSize = (long) tdfConfig.defaultSegmentSize + kGcmIvSize + AesGcm.GCM_TAG_LENGTH; TDFWriter tdfWriter = new TDFWriter(outputStream); ByteArrayOutputStream aggregateHash = new ByteArrayOutputStream(); diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java index e9760e28..3fbb3eac 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java @@ -21,7 +21,6 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; -import java.io.OutputStream; import java.nio.charset.StandardCharsets; import java.security.KeyPair; import java.security.cert.X509Certificate; @@ -705,9 +704,9 @@ public void testCreatingTDFWithMultipleSegments() throws Exception { var tdfObject = tdf.createTDF(plainTextInputStream, tdfOutputStream, config); var segments = tdfObject.getManifest().encryptionInformation.integrityInformation.segments; - assertThat(segments.size()) + assertThat(segments) .withFailMessage("test needs more than one segment to be meaningful") - .isGreaterThan(1); + .hasSizeGreaterThan(1); // payload segments start at IV 1 (IV 0 is reserved for the metadata) and // increment by one for every segment diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java index 2e38779a..97b3fec8 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java @@ -217,7 +217,7 @@ private static byte[] keepOnlyTheEntryCountSentinel(byte[] archive) { private static void assertReadsEveryEntry(byte[] archive) throws IOException { try (var channel = new SeekableInMemoryByteChannel(archive)) { var reader = new ZipReader(channel); - assertThat(reader.getEntries().size()).isEqualTo(3); + assertThat(reader.getEntries()).hasSize(3); assertThat(readEntry(reader, "small.txt")).isEqualTo("tiny"); assertThat(readEntry(reader, "0.payload")).isEqualTo(PAYLOAD); assertThat(readEntry(reader, "0.manifest.json")).isEqualTo(MANIFEST); From c2b90b60d6f686dfdbe59feccb72ab0b168c2d95 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Wed, 2 Sep 2026 08:09:28 -0400 Subject: [PATCH 5/6] refactor(sdk): extract the local header parsing out of ZipReader.getData Pure refactor, no behavior change. getData was doing three things: checking the entry's local header offset, parsing the header, and building the InputStream over the entry's bytes. The first two move into private helpers on Entry, which also drops getData back under the cognitive complexity limit that the offset check pushed it over. Signed-off-by: Dave Mihalcik --- .../io/opentdf/platform/sdk/ZipReader.java | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java b/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java index 2366f2b8..8f303d13 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java @@ -189,11 +189,24 @@ public String getName() { return fileName; } - public InputStream getData() throws IOException { + /** + * Checks that this entry's local header offset points inside the archive, so a corrupt + * or truncated central directory fails here rather than at an arbitrary position. + */ + private void checkOffsetToLocalHeader() throws IOException { if (offsetToLocalHeader < 0 || offsetToLocalHeader >= zipChannel.size()) { throw new InvalidZipException("local header offset out of range for entry [" + fileName + "]: " + offsetToLocalHeader); } + } + + /** + * Reads this entry's local file header and returns the offset of the first byte of its + * data. Leaves the channel positioned within the header rather than at the returned + * offset, because the filename and extra field are skipped by arithmetic. + */ + private long findStartOfData() throws IOException { + checkOffsetToLocalHeader(); zipChannel.position(offsetToLocalHeader); Integer signature = readInteger(); if (signature == null || signature != LOCAL_FILE_HEADER_SIGNATURE) { @@ -212,7 +225,11 @@ public InputStream getData() throws IOException { int filenameLength = readUnsignedShort(); int extrafieldLength = readUnsignedShort(); - final long startPosition = zipChannel.position() + filenameLength + extrafieldLength; + return zipChannel.position() + filenameLength + extrafieldLength; + } + + public InputStream getData() throws IOException { + final long startPosition = findStartOfData(); final long endPosition = startPosition + fileSize; final ByteBuffer buf = ByteBuffer.allocate(1); return new InputStream() { From 6ed40df3d2c1967d4cad0c84e10e2510ef7686d4 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Wed, 2 Sep 2026 17:43:14 -0400 Subject: [PATCH 6/6] fix(sdk): advance single-byte ZIP entry reads Signed-off-by: Dave Mihalcik --- .../io/opentdf/platform/sdk/ZipReader.java | 1 + .../opentdf/platform/sdk/ZipReaderTest.java | 27 ++++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java b/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java index 8f303d13..576ba5de 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java @@ -240,6 +240,7 @@ public int read() throws IOException { return -1; } setChannelPosition(); + buf.clear(); while (buf.hasRemaining()) { if (zipChannel.read(buf) <= 0) { return -1; diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java index 97b3fec8..fa2014bd 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java @@ -94,6 +94,31 @@ public void testReadingAFileWrittenUsingCommons() throws IOException { } } + @Test + public void testSingleByteReadAdvancesThroughEntry() throws IOException { + byte[] expected = "contents with distinct bytes".getBytes(StandardCharsets.UTF_8); + SeekableInMemoryByteChannel outputChannel = new SeekableInMemoryByteChannel(); + ZipArchiveOutputStream zip = new ZipArchiveOutputStream(outputChannel); + ZipArchiveEntry zipEntry = new ZipArchiveEntry("entry"); + zipEntry.setMethod(0); + zip.putArchiveEntry(zipEntry); + zip.write(expected); + zip.closeArchiveEntry(); + zip.close(); + + var reader = new ZipReader(new SeekableInMemoryByteChannel(outputChannel.array())); + var entry = reader.getEntries().get(0); + var actual = new ByteArrayOutputStream(); + try (var data = entry.getData()) { + int next; + while ((next = data.read()) != -1) { + actual.write(next); + } + } + + assertThat(actual.toByteArray()).isEqualTo(expected); + } + @Test public void testReadingAndWritingRandomFiles() throws IOException { Random r = new Random(); @@ -248,4 +273,4 @@ private static String readEntry(ZipFile zip, String name) throws IOException { zip.getInputStream(zip.getEntry(name)).transferTo(data); return data.toString(StandardCharsets.UTF_8); } -} \ No newline at end of file +}