From a2409b07ae66fbcb55c7a27db235738640858987 Mon Sep 17 00:00:00 2001 From: Mike Jensen Date: Mon, 25 Nov 2024 15:11:15 -0700 Subject: [PATCH 1/9] ZipReader: Fix for possible protocol attacks This change fixes a couple possible protocol attacks: * Possible NullPointerExceptions from `readLong/Int/Short` when a full value could not be read. This is assumed to be unexpected, and may not be handled by users. Instead `InvalidZipException` is now thrown. * A permutation of the above is also possible when reading the Signature. The behavior was changed to defer to the existing signature validation failure (logging and existing InvalidZipException). * Loops where a partial read is handled (see example reading in the filename) could result in a tight loop thread if content is shorter than the defined length. The logic now expects a blocking input which will only return a zero value if the content has reached the end. A premature end will result in an `EOFException`. --- .../io/opentdf/platform/sdk/ZipReader.java | 48 +++++++++++-------- 1 file changed, 29 insertions(+), 19 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 cb6d5ffd..cf0b5772 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java @@ -3,6 +3,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.EOFException; import java.io.IOException; import java.io.InputStream; import java.nio.ByteBuffer; @@ -25,17 +26,17 @@ public class ZipReader { public static final int ZIP64_END_OF_CENTRAL_DIRECTORY_LOCATOR_SIZE = 20; final ByteBuffer longBuf = ByteBuffer.allocate(Long.BYTES).order(ByteOrder.LITTLE_ENDIAN); - private Long readLong() throws IOException { + private long readLong() throws IOException { longBuf.clear(); if (this.zipChannel.read(longBuf) != 8) { - return null; + throw new InvalidZipException("Expected long value"); } longBuf.flip(); return longBuf.getLong(); } final ByteBuffer intBuf = ByteBuffer.allocate(Integer.BYTES).order(ByteOrder.LITTLE_ENDIAN); - private Integer readInt() throws IOException { + private Integer readInteger() throws IOException { intBuf.clear(); if (this.zipChannel.read(intBuf) != 4) { return null; @@ -43,13 +44,20 @@ private Integer readInt() throws IOException { intBuf.flip(); return intBuf.getInt(); } + private int readInt() throws IOException { + Integer result = readInteger(); + if (result == null) { + throw new InvalidZipException("Expected int value"); + } + return result.intValue(); + } final ByteBuffer shortBuf = ByteBuffer.allocate(Short.BYTES).order(ByteOrder.LITTLE_ENDIAN); - private Short readShort() throws IOException { + private short readShort() throws IOException { shortBuf.clear(); if (this.zipChannel.read(shortBuf) != 2) { - return null; + throw new InvalidZipException("Expected short value"); } shortBuf.flip(); return shortBuf.getShort(); @@ -79,8 +87,8 @@ CentralDirectoryRecord readEndOfCentralDirectory() throws IOException { while (eoCDRStart >= 0) { zipChannel.position(eoCDRStart); - int signature = readInt(); - if (signature == END_OF_CENTRAL_DIRECTORY_SIGNATURE) { + Integer signature = readInteger(); + if (signature == null || signature == END_OF_CENTRAL_DIRECTORY_SIGNATURE) { if (logger.isDebugEnabled()) { logger.debug("Found end of central directory signature at {}", zipChannel.position() - Integer.BYTES); } @@ -113,8 +121,8 @@ CentralDirectoryRecord readEndOfCentralDirectory() throws IOException { private CentralDirectoryRecord extractZIP64CentralDirectoryInfo() throws IOException { // buffer's position at the start of the Central Directory - int signature = readInt(); - if (signature != ZIP64_END_OF_CENTRAL_DIRECTORY_LOCATOR_SIGNATURE) { + Integer signature = readInteger(); + if (signature == null || signature != ZIP64_END_OF_CENTRAL_DIRECTORY_LOCATOR_SIGNATURE) { throw new InvalidZipException("Invalid Zip64 End of Central Directory Record Signature"); } @@ -157,7 +165,8 @@ public String getName() { public InputStream getData() throws IOException { zipChannel.position(offsetToLocalHeader); - if (readInt() != LOCAL_FILE_HEADER_SIGNATURE) { + Integer signature = readInteger(); + if (signature == null || signature != LOCAL_FILE_HEADER_SIGNATURE) { throw new InvalidZipException("Invalid Local Header Signature"); } zipChannel.position(zipChannel.position() @@ -184,8 +193,8 @@ public int read() throws IOException { return -1; } setChannelPosition(); - while (buf.position() != buf.capacity()) { - if (zipChannel.read(buf) < 0) { + while (buf.hasRemaining()) { + if (zipChannel.read(buf) <= 0) { return -1; } } @@ -222,8 +231,8 @@ public int read(byte[] b, int off, int len) throws IOException { } } public Entry readCentralDirectoryFileHeader() throws IOException { - int signature = readInt(); - if (signature != CENTRAL_FILE_HEADER_SIGNATURE) { + Integer signature = readInteger(); + if (signature == null || signature != CENTRAL_FILE_HEADER_SIGNATURE) { throw new InvalidZipException("Invalid Central Directory File Header Signature"); } short versionMadeBy = readShort(); @@ -244,8 +253,10 @@ public Entry readCentralDirectoryFileHeader() throws IOException { long relativeOffsetOfLocalHeader = readInt(); ByteBuffer fileName = ByteBuffer.allocate(fileNameLength); - while (fileName.position() != fileName.capacity()) { - zipChannel.read(fileName); + while (fileName.hasRemaining()) { + if (zipChannel.read(fileName) <= 0) { + throw new EOFException("Unexpected EOF when reading filename of length: " + fileNameLength); + } } // Parse the extra field @@ -277,7 +288,6 @@ public Entry readCentralDirectoryFileHeader() throws IOException { return new Entry(fileName.array(), relativeOffsetOfLocalHeader, uncompressedSize); } - public ZipReader(SeekableByteChannel channel) throws IOException { zipChannel = channel; var centralDirectoryRecord = readEndOfCentralDirectory(); @@ -286,11 +296,11 @@ public ZipReader(SeekableByteChannel channel) throws IOException { entries.add(readCentralDirectoryFileHeader()); } } - + final SeekableByteChannel zipChannel; final ArrayList entries = new ArrayList<>(); public List getEntries() { return entries; } -} \ No newline at end of file +} From 46388f48e5ff4ed202a6e2ec5f6a4ed85298b8c0 Mon Sep 17 00:00:00 2001 From: Mike Jensen Date: Mon, 25 Nov 2024 15:23:32 -0700 Subject: [PATCH 2/9] Minor memory optimization to initialize ArrayList with known sizes When the list size is known initializing at that value reduces the minor memory overhead of expanding and copying the underline array. --- sdk/src/main/java/io/opentdf/platform/sdk/NanoTDF.java | 2 +- sdk/src/main/java/io/opentdf/platform/sdk/SDKBuilder.java | 2 +- sdk/src/main/java/io/opentdf/platform/sdk/TDF.java | 7 +++---- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/NanoTDF.java b/sdk/src/main/java/io/opentdf/platform/sdk/NanoTDF.java index f893762a..5f92069c 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/NanoTDF.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/NanoTDF.java @@ -252,7 +252,7 @@ PolicyObject createPolicyObject(List attributes) { PolicyObject policyObject = new PolicyObject(); policyObject.body = new PolicyObject.Body(); policyObject.uuid = UUID.randomUUID().toString(); - policyObject.body.dataAttributes = new ArrayList<>(); + policyObject.body.dataAttributes = new ArrayList<>(attributes.size()); policyObject.body.dissem = new ArrayList<>(); for (String attribute : attributes) { diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/SDKBuilder.java b/sdk/src/main/java/io/opentdf/platform/sdk/SDKBuilder.java index 8c6b1804..9f4e2cd9 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/SDKBuilder.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/SDKBuilder.java @@ -74,7 +74,7 @@ public SDKBuilder sslFactoryFromDirectory(String certsDirPath) throws Exception File certsDir = new File(certsDirPath); File[] certFiles = certsDir.listFiles((dir, name) -> name.endsWith(".pem") || name.endsWith(".crt")); logger.info("Loading certificates from: " + certsDir.getAbsolutePath()); - List certStreams = new ArrayList<>(); + List certStreams = new ArrayList<>(certFiles.length); for (File certFile : certFiles) { certStreams.add(new FileInputStream(certFile)); } 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 124ebe0f..72892ae1 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java @@ -188,7 +188,7 @@ PolicyObject createPolicyObject(List attributes PolicyObject policyObject = new PolicyObject(); policyObject.body = new PolicyObject.Body(); policyObject.uuid = UUID.randomUUID().toString(); - policyObject.body.dataAttributes = new ArrayList<>(); + policyObject.body.dataAttributes = new ArrayList<>(attributes.size()); policyObject.body.dissem = new ArrayList<>(); for (Autoconfigure.AttributeValueFQN attribute : attributes) { @@ -208,7 +208,6 @@ private void prepareManifest(Config.TDFConfig tdfConfig, SDK.KAS kas) { PolicyObject policyObject = createPolicyObject(tdfConfig.attributes); String base64PolicyObject = encoder .encodeToString(gson.toJson(policyObject).getBytes(StandardCharsets.UTF_8)); - List symKeys = new ArrayList<>(); Map latestKASInfo = new HashMap<>(); if (tdfConfig.splitPlan == null || tdfConfig.splitPlan.isEmpty()) { // Default split plan: Split keys across all KASes @@ -261,6 +260,7 @@ private void prepareManifest(Config.TDFConfig tdfConfig, SDK.KAS kas) { } } + List symKeys = new ArrayList<>(splitIDs.size()); for (String splitID : splitIDs) { // Symmetric key byte[] symKey = new byte[GCM_KEY_SIZE]; @@ -520,8 +520,7 @@ public TDFObject createTDF(InputStream payload, tdfObject.manifest.payload.url = TDFWriter.TDF_PAYLOAD_FILE_NAME; tdfObject.manifest.payload.isEncrypted = true; - List signedAssertions = new ArrayList<>(); - + List signedAssertions = new ArrayList<>(tdfConfig.assertionConfigList.size()); for (var assertionConfig : tdfConfig.assertionConfigList) { var assertion = new Manifest.Assertion(); assertion.id = assertionConfig.id; From cf0e71a893508189c5995a0fa4d31c92106abd7e Mon Sep 17 00:00:00 2001 From: Mike Jensen Date: Mon, 25 Nov 2024 16:45:03 -0700 Subject: [PATCH 3/9] TDFTest: Remove unecessary `attributeGrpcStub` This mocked class is not needed due to TDF only being used with auto configure set to `false`. To simplify this test the mock was removed and instead null is passed in to validate it's not used. --- .../java/io/opentdf/platform/sdk/TDFTest.java | 74 +++---------------- 1 file changed, 10 insertions(+), 64 deletions(-) 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 acd1fd15..35416f37 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java @@ -35,13 +35,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; public class TDFTest { - - @BeforeEach - public void setup() { - attributeGrpcStub = mock(AttributesServiceGrpc.AttributesServiceFutureStub.class); - } - - private static SDK.KAS kas = new SDK.KAS() { + protected static SDK.KAS kas = new SDK.KAS() { @Override public void close() { } @@ -84,8 +78,6 @@ public KASKeyCache getKeyCache() { } }; - AttributesServiceGrpc.AttributesServiceFutureStub attributeGrpcStub; - private static ArrayList keypairs = new ArrayList<>(); @BeforeAll @@ -97,12 +89,6 @@ static void createKeypairs() { @Test void testSimpleTDFEncryptAndDecrypt() throws Exception { - - ListenableFuture resp1 = mock(ListenableFuture.class); - lenient().when(resp1.get()).thenReturn(GetAttributeValuesByFqnsResponse.newBuilder().build()); - lenient().when(attributeGrpcStub.getAttributeValuesByFqns(any(GetAttributeValuesByFqnsRequest.class))) - .thenReturn(resp1); - SecureRandom secureRandom = new SecureRandom(); byte[] key = new byte[32]; secureRandom.nextBytes(key); @@ -130,7 +116,7 @@ void testSimpleTDFEncryptAndDecrypt() throws Exception { ByteArrayOutputStream tdfOutputStream = new ByteArrayOutputStream(); TDF tdf = new TDF(); - tdf.createTDF(plainTextInputStream, tdfOutputStream, config, kas, attributeGrpcStub); + tdf.createTDF(plainTextInputStream, tdfOutputStream, config, kas, null); var assertionVerificationKeys = new Config.AssertionVerificationKeys(); assertionVerificationKeys.defaultKey = new AssertionConfig.AssertionKey(AssertionConfig.AssertionKeyAlg.HS256, @@ -139,6 +125,7 @@ void testSimpleTDFEncryptAndDecrypt() throws Exception { var unwrappedData = new ByteArrayOutputStream(); Config.TDFReaderConfig readerConfig = Config.newTDFReaderConfig( Config.withAssertionVerificationKeys(assertionVerificationKeys)); + var reader = tdf.loadTDF(new SeekableInMemoryByteChannel(tdfOutputStream.toByteArray()), kas, readerConfig); assertThat(reader.getManifest().payload.mimeType).isEqualTo("application/octet-stream"); @@ -158,12 +145,6 @@ void testSimpleTDFEncryptAndDecrypt() throws Exception { @Test void testSimpleTDFWithAssertionWithRS256() throws Exception { - - ListenableFuture resp1 = mock(ListenableFuture.class); - lenient().when(resp1.get()).thenReturn(GetAttributeValuesByFqnsResponse.newBuilder().build()); - lenient().when(attributeGrpcStub.getAttributeValuesByFqns(any(GetAttributeValuesByFqnsRequest.class))) - .thenReturn(resp1); - String assertion1Id = "assertion1"; var keypair = CryptoUtils.generateRSAKeypair(); var assertionConfig = new AssertionConfig(); @@ -188,7 +169,7 @@ void testSimpleTDFWithAssertionWithRS256() throws Exception { ByteArrayOutputStream tdfOutputStream = new ByteArrayOutputStream(); TDF tdf = new TDF(); - tdf.createTDF(plainTextInputStream, tdfOutputStream, config, kas, attributeGrpcStub); + tdf.createTDF(plainTextInputStream, tdfOutputStream, config, kas, null); var assertionVerificationKeys = new Config.AssertionVerificationKeys(); assertionVerificationKeys.keys.put(assertion1Id, @@ -208,12 +189,6 @@ void testSimpleTDFWithAssertionWithRS256() throws Exception { @Test void testWithAssertionVerificationDisabled() throws Exception { - - ListenableFuture resp1 = mock(ListenableFuture.class); - lenient().when(resp1.get()).thenReturn(GetAttributeValuesByFqnsResponse.newBuilder().build()); - lenient().when(attributeGrpcStub.getAttributeValuesByFqns(any(GetAttributeValuesByFqnsRequest.class))) - .thenReturn(resp1); - String assertion1Id = "assertion1"; var keypair = CryptoUtils.generateRSAKeypair(); var assertionConfig = new AssertionConfig(); @@ -238,7 +213,7 @@ void testWithAssertionVerificationDisabled() throws Exception { ByteArrayOutputStream tdfOutputStream = new ByteArrayOutputStream(); TDF tdf = new TDF(); - tdf.createTDF(plainTextInputStream, tdfOutputStream, config, kas, attributeGrpcStub); + tdf.createTDF(plainTextInputStream, tdfOutputStream, config, kas, null); var assertionVerificationKeys = new Config.AssertionVerificationKeys(); assertionVerificationKeys.keys.put(assertion1Id, @@ -263,12 +238,6 @@ void testWithAssertionVerificationDisabled() throws Exception { } @Test void testSimpleTDFWithAssertionWithHS256() throws Exception { - - ListenableFuture resp1 = mock(ListenableFuture.class); - lenient().when(resp1.get()).thenReturn(GetAttributeValuesByFqnsResponse.newBuilder().build()); - lenient().when(attributeGrpcStub.getAttributeValuesByFqns(any(GetAttributeValuesByFqnsRequest.class))) - .thenReturn(resp1); - String assertion1Id = "assertion1"; var assertionConfig1 = new AssertionConfig(); assertionConfig1.id = assertion1Id; @@ -301,7 +270,7 @@ void testSimpleTDFWithAssertionWithHS256() throws Exception { ByteArrayOutputStream tdfOutputStream = new ByteArrayOutputStream(); TDF tdf = new TDF(); - tdf.createTDF(plainTextInputStream, tdfOutputStream, config, kas, attributeGrpcStub); + tdf.createTDF(plainTextInputStream, tdfOutputStream, config, kas, null); var unwrappedData = new ByteArrayOutputStream(); var reader = tdf.loadTDF(new SeekableInMemoryByteChannel(tdfOutputStream.toByteArray()), @@ -335,12 +304,6 @@ void testSimpleTDFWithAssertionWithHS256() throws Exception { @Test void testSimpleTDFWithAssertionWithHS256Failure() throws Exception { - - ListenableFuture resp1 = mock(ListenableFuture.class); - lenient().when(resp1.get()).thenReturn(GetAttributeValuesByFqnsResponse.newBuilder().build()); - lenient().when(attributeGrpcStub.getAttributeValuesByFqns(any(GetAttributeValuesByFqnsRequest.class))) - .thenReturn(resp1); - // var keypair = CryptoUtils.generateRSAKeypair(); SecureRandom secureRandom = new SecureRandom(); byte[] key = new byte[32]; @@ -368,7 +331,7 @@ void testSimpleTDFWithAssertionWithHS256Failure() throws Exception { ByteArrayOutputStream tdfOutputStream = new ByteArrayOutputStream(); TDF tdf = new TDF(); - tdf.createTDF(plainTextInputStream, tdfOutputStream, config, kas, attributeGrpcStub); + tdf.createTDF(plainTextInputStream, tdfOutputStream, config, kas, null); byte[] notkey = new byte[32]; secureRandom.nextBytes(notkey); @@ -391,12 +354,6 @@ void testSimpleTDFWithAssertionWithHS256Failure() throws Exception { @Test public void testCreatingTDFWithMultipleSegments() throws Exception { - - ListenableFuture resp1 = mock(ListenableFuture.class); - lenient().when(resp1.get()).thenReturn(GetAttributeValuesByFqnsResponse.newBuilder().build()); - lenient().when(attributeGrpcStub.getAttributeValuesByFqns(any(GetAttributeValuesByFqnsRequest.class))) - .thenReturn(resp1); - var random = new Random(); Config.TDFConfig config = Config.newTDFConfig( @@ -411,7 +368,7 @@ public void testCreatingTDFWithMultipleSegments() throws Exception { var plainTextInputStream = new ByteArrayInputStream(data); var tdfOutputStream = new ByteArrayOutputStream(); var tdf = new TDF(); - tdf.createTDF(plainTextInputStream, tdfOutputStream, config, kas, attributeGrpcStub); + tdf.createTDF(plainTextInputStream, tdfOutputStream, config, kas, null); var unwrappedData = new ByteArrayOutputStream(); var reader = tdf.loadTDF(new SeekableInMemoryByteChannel(tdfOutputStream.toByteArray()), kas); reader.readPayload(unwrappedData); @@ -424,11 +381,6 @@ public void testCreatingTDFWithMultipleSegments() throws Exception { @Test public void testCreatingTooLargeTDF() throws Exception { - ListenableFuture resp1 = mock(ListenableFuture.class); - lenient().when(resp1.get()).thenReturn(GetAttributeValuesByFqnsResponse.newBuilder().build()); - lenient().when(attributeGrpcStub.getAttributeValuesByFqns(any(GetAttributeValuesByFqnsRequest.class))) - .thenReturn(resp1); - var random = new Random(); var maxSize = random.nextInt(1024); var numReturned = new AtomicInteger(0); @@ -468,7 +420,7 @@ public void write(byte[] b, int off, int len) { Config.withKasInformation(getKASInfos()), Config.withSegmentSize(1 + random.nextInt(128))); assertThrows(TDF.DataSizeNotSupported.class, - () -> tdf.createTDF(is, os, tdfConfig, kas, attributeGrpcStub), + () -> tdf.createTDF(is, os, tdfConfig, kas, null), "didn't throw an exception when we created TDF that was too large"); assertThat(numReturned.get()) .withFailMessage("test returned the wrong number of bytes") @@ -477,12 +429,6 @@ public void write(byte[] b, int off, int len) { @Test public void testCreateTDFWithMimeType() throws Exception { - - ListenableFuture resp1 = mock(ListenableFuture.class); - lenient().when(resp1.get()).thenReturn(GetAttributeValuesByFqnsResponse.newBuilder().build()); - lenient().when(attributeGrpcStub.getAttributeValuesByFqns(any(GetAttributeValuesByFqnsRequest.class))) - .thenReturn(resp1); - final String mimeType = "application/pdf"; Config.TDFConfig config = Config.newTDFConfig( @@ -495,7 +441,7 @@ public void testCreateTDFWithMimeType() throws Exception { ByteArrayOutputStream tdfOutputStream = new ByteArrayOutputStream(); TDF tdf = new TDF(); - tdf.createTDF(plainTextInputStream, tdfOutputStream, config, kas, attributeGrpcStub); + tdf.createTDF(plainTextInputStream, tdfOutputStream, config, kas, null); var reader = tdf.loadTDF(new SeekableInMemoryByteChannel(tdfOutputStream.toByteArray()), kas); assertThat(reader.getManifest().payload.mimeType).isEqualTo(mimeType); From 2f8147a0b1339f17cdbc26946df6432b051a95d2 Mon Sep 17 00:00:00 2001 From: Mike Jensen Date: Tue, 26 Nov 2024 12:21:55 -0700 Subject: [PATCH 4/9] Validate TDF Manifest to prevent possible NullPointerException conditions This commit validates the fields were read from the Manifest before the TDF is read. This results in convering previous NullPointerExceptions into `IllegalArgumentExceptions` with a message that indicates what field had an unexpected state. --- .../io/opentdf/platform/sdk/Manifest.java | 41 +++++++++++++++++-- .../java/io/opentdf/platform/sdk/TDF.java | 4 +- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/Manifest.java b/sdk/src/main/java/io/opentdf/platform/sdk/Manifest.java index 479d7b0e..b72fad64 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/Manifest.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/Manifest.java @@ -260,7 +260,7 @@ static public class Payload { public String url; public String protocol; public String mimeType; - public Boolean isEncrypted; + public boolean isEncrypted; @Override public boolean equals(Object o) { @@ -473,8 +473,43 @@ public Manifest deserialize(JsonElement json, Type typeOfT, JsonDeserializationC } } - private static Manifest readManifest(Reader reader) { - return gson.fromJson(reader, Manifest.class); + protected static Manifest readManifest(Reader reader) { + Manifest result = gson.fromJson(reader, Manifest.class); + if (result == null) { + throw new IllegalArgumentException("Manifest is null"); + } else if (result.payload == null) { + throw new IllegalArgumentException("Manifest with null payload"); + } else if (result.encryptionInformation == null) { + throw new IllegalArgumentException("Manifest with null encryptionInformation"); + } else if (result.encryptionInformation.integrityInformation == null) { + throw new IllegalArgumentException("Manifest with null integrityInformation"); + } else if (result.encryptionInformation.integrityInformation.segments == null) { + throw new IllegalArgumentException("Manifest with invalid integrityInformation"); + } else if (result.encryptionInformation.integrityInformation.rootSignature == null) { + throw new IllegalArgumentException("Manifest with null rootSignature"); + } else if (result.encryptionInformation.integrityInformation.rootSignature.algorithm == null + || result.encryptionInformation.integrityInformation.rootSignature.signature == null) { + throw new IllegalArgumentException("Manifest with invalid rootSignature"); + } else if (result.encryptionInformation.integrityInformation.segments == null) { + throw new IllegalArgumentException("Manifest with null segments"); + } else if (result.encryptionInformation.keyAccessObj == null) { + throw new IllegalArgumentException("Manifest with null keyAccessObj"); + } else if (result.encryptionInformation.policy == null) { + throw new IllegalArgumentException("Manifest with null policy"); + } + + for (Manifest.Segment segment : result.encryptionInformation.integrityInformation.segments) { + if (segment == null || segment.hash == null) { + throw new IllegalArgumentException("Invalid integrity segment"); + } + } + for (Manifest.KeyAccess keyAccess : result.encryptionInformation.keyAccessObj) { + if (keyAccess == null) { + throw new IllegalArgumentException("Invalid null KeyAccess in manifest"); + } + } + + return result; } static PolicyObject readPolicyObject(Reader reader) { 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 72892ae1..e869e1fe 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java @@ -23,6 +23,7 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.io.OutputStream; +import java.io.StringReader; import java.nio.channels.SeekableByteChannel; import java.nio.charset.StandardCharsets; import java.security.*; @@ -584,7 +585,8 @@ public Reader loadTDF(SeekableByteChannel tdf, SDK.KAS kas, TDFReader tdfReader = new TDFReader(tdf); String manifestJson = tdfReader.manifest(); - Manifest manifest = gson.fromJson(manifestJson, Manifest.class); + // use Manifest.readManifest in order to validate the Manifest input + Manifest manifest = Manifest.readManifest(new StringReader(manifestJson)); byte[] payloadKey = new byte[GCM_KEY_SIZE]; String unencryptedMetadata = null; From 1552cc0514e6acaadcd4101af4b186aedf09bb3c Mon Sep 17 00:00:00 2001 From: Mike Jensen Date: Tue, 26 Nov 2024 15:26:58 -0700 Subject: [PATCH 5/9] PolicyInfo: Prevent policy length overflow, resulting in a negative length --- .../main/java/io/opentdf/platform/sdk/nanotdf/PolicyInfo.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/PolicyInfo.java b/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/PolicyInfo.java index c4b93f34..78b96aea 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/PolicyInfo.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/nanotdf/PolicyInfo.java @@ -29,7 +29,8 @@ public PolicyInfo(ByteBuffer buffer, ECCMode eccMode) { byte[] policyLengthBuf = new byte[Short.BYTES]; buffer.get(policyLengthBuf); - short policyLength = ByteBuffer.wrap(policyLengthBuf).getShort(); + // read short value into int to prevent possible overflow resulting in negative length + int policyLength = ByteBuffer.wrap(policyLengthBuf).getShort(); if (this.type == NanoTDFType.PolicyType.EMBEDDED_POLICY_PLAIN_TEXT || this.type == NanoTDFType.PolicyType.EMBEDDED_POLICY_ENCRYPTED) { From 0cb040d17e1b5d865e48f70e8c26047e3d985ccf Mon Sep 17 00:00:00 2001 From: Mike Jensen Date: Tue, 26 Nov 2024 15:27:20 -0700 Subject: [PATCH 6/9] TDF: Set min and max limits for segment sizes This matches the protections introduced in the go SDK PR: https://github.com/opentdf/platform/pull/1536 --- sdk/src/main/java/io/opentdf/platform/sdk/Config.java | 8 ++++++++ sdk/src/main/java/io/opentdf/platform/sdk/TDF.java | 6 ++++++ 2 files changed, 14 insertions(+) diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/Config.java b/sdk/src/main/java/io/opentdf/platform/sdk/Config.java index 7b7d824f..8f1e7da3 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/Config.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/Config.java @@ -21,6 +21,8 @@ public class Config { public static final int TDF3_KEY_SIZE = 2048; public static final int DEFAULT_SEGMENT_SIZE = 2 * 1024 * 1024; // 2mb + public static final int MAX_SEGMENT_SIZE = DEFAULT_SEGMENT_SIZE * 2; + public static final int MIN_SEGMENT_SIZE = 16 * 1024; public static final String KAS_PUBLIC_KEY_PATH = "/kas_public_key"; public static final String DEFAULT_MIME_TYPE = "application/octet-stream"; public static final int MAX_COLLECTION_ITERATION = (1 << 24) - 1; @@ -228,6 +230,12 @@ public static Consumer withMetaData(String metaData) { } public static Consumer withSegmentSize(int size) { + if (size > MAX_SEGMENT_SIZE) { + throw new IllegalArgumentException("Segment size " + size + " exceeds the maximum " + MAX_SEGMENT_SIZE); + } else if (size < MIN_SEGMENT_SIZE) { + throw new IllegalArgumentException("Segment size " + size + " is under the minimum " + MIN_SEGMENT_SIZE); + } + return (TDFConfig config) -> config.defaultSegmentSize = size; } 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 e869e1fe..c24628e2 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java @@ -359,6 +359,12 @@ public void readPayload(OutputStream outputStream) throws TDFReadFailed, MessageDigest digest = MessageDigest.getInstance("SHA-256"); for (Manifest.Segment segment : manifest.encryptionInformation.integrityInformation.segments) { + if (segment.encryptedSegmentSize > Config.MAX_SEGMENT_SIZE) { + throw new IllegalStateException("Segment size " + segment.encryptedSegmentSize + " exceeded limit " + Config.MAX_SEGMENT_SIZE); + } else if (segment.encryptedSegmentSize < Config.MIN_SEGMENT_SIZE) { + throw new IllegalStateException("Segment size " + segment.encryptedSegmentSize + " is under minimum " + Config.MIN_SEGMENT_SIZE); + } + byte[] readBuf = new byte[(int) segment.encryptedSegmentSize]; int bytesRead = tdfReader.readPayloadBytes(readBuf); From 29b472d3164e65d5a321d55d48c0953386cbb651 Mon Sep 17 00:00:00 2001 From: Mike Jensen Date: Wed, 27 Nov 2024 11:44:19 -0700 Subject: [PATCH 7/9] Add fuzz testing that was used to find previous fixes This fuzz testing and seed corpus helped validate for protocol flaws in decoding TDF's. This testing is time consuming, and Jazzer sometimes has some weird IO blocking behavior that is not actually indicative of a flaw. For that reason this is not part of CI, and instead is run through `fuzz.sh` when needed. --- sdk/fuzz.sh | 11 ++ sdk/pom.xml | 124 +++++++++++++++++- .../java/io/opentdf/platform/sdk/Fuzzing.java | 69 ++++++++++ .../io/opentdf/platform/sdk/NanoTDFTest.java | 2 +- .../opentdf/platform/sdk/ZipReaderTest.java | 38 ++++-- .../sdk/FuzzingInputs/fuzzNanoTDF/sample.ntdf | Bin 0 -> 361 bytes .../fuzzTDF/crash-InvalidManifest-1 | Bin 0 -> 2017 bytes .../fuzzTDF/crash-InvalidManifest-2 | Bin 0 -> 2017 bytes .../fuzzTDF/crash-InvalidManifest-3 | Bin 0 -> 2017 bytes .../fuzzTDF/crash-InvalidManifest-4 | Bin 0 -> 2017 bytes .../crash-InvalidManifest-NullKeyAccessObj | Bin 0 -> 2017 bytes .../fuzzTDF/crash-InvalidManifest-NullSegment | Bin 0 -> 2017 bytes .../sdk/FuzzingInputs/fuzzTDF/sample.tdf | Bin 0 -> 2017 bytes .../fuzzZipRead/crash-NullSignature | Bin 0 -> 1371 bytes ...h-f39ad8416aef7cf275f84683aaa0efd15f24272a | Bin 0 -> 99 bytes .../FuzzingInputs/fuzzZipRead/sample.txt.tdf | Bin 0 -> 1754 bytes 16 files changed, 230 insertions(+), 14 deletions(-) create mode 100755 sdk/fuzz.sh create mode 100644 sdk/src/test/java/io/opentdf/platform/sdk/Fuzzing.java create mode 100644 sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzNanoTDF/sample.ntdf create mode 100644 sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzTDF/crash-InvalidManifest-1 create mode 100644 sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzTDF/crash-InvalidManifest-2 create mode 100644 sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzTDF/crash-InvalidManifest-3 create mode 100644 sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzTDF/crash-InvalidManifest-4 create mode 100644 sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzTDF/crash-InvalidManifest-NullKeyAccessObj create mode 100644 sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzTDF/crash-InvalidManifest-NullSegment create mode 100644 sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzTDF/sample.tdf create mode 100644 sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzZipRead/crash-NullSignature create mode 100644 sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzZipRead/crash-f39ad8416aef7cf275f84683aaa0efd15f24272a create mode 100644 sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzZipRead/sample.txt.tdf diff --git a/sdk/fuzz.sh b/sdk/fuzz.sh new file mode 100755 index 00000000..88a803ae --- /dev/null +++ b/sdk/fuzz.sh @@ -0,0 +1,11 @@ +#!/bin/bash +set -e + +tests=("fuzzNanoTDF", "fuzzTDF", "fuzzZipRead") +base_seed_dir="src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/" + +for test in "${tests[@]}"; do + seed_dir="${base_seed_dir}${test}" + echo "Running $test fuzzing with seeds from $seed_dir" + mvn verify -P fuzz -Djazzer.testDir=$seed_dir +done diff --git a/sdk/pom.xml b/sdk/pom.xml index 678c78b0..6685f513 100644 --- a/sdk/pom.xml +++ b/sdk/pom.xml @@ -9,6 +9,10 @@ 0.7.5 jar + + 0.22.1 + https://github.com/CodeIntelligenceTesting/jazzer/releases/download/v${jazzer.version} + @@ -121,6 +125,18 @@ 4.13.2 test + + com.code-intelligence + jazzer-api + ${jazzer.version} + test + + + com.code-intelligence + jazzer-junit + ${jazzer.version} + test + org.apache.commons commons-compress @@ -307,4 +323,110 @@ - \ No newline at end of file + + + + fuzz + + false + + + true + + + + + + org.apache.maven.plugins + maven-antrun-plugin + 3.1.0 + + + download-and-unpack-jazzer + process-test-classes + + + + + + + + + + + + + + + + + + + run + + + + + + + + org.apache.maven.plugins + maven-dependency-plugin + 3.4.0 + + + copy-dependencies + process-test-classes + + copy-dependencies + + + ${project.build.directory}/dependency-jars + test + + + + + + + + org.apache.maven.plugins + maven-antrun-plugin + 3.1.0 + + + run-jazzer-fuzzing + verify + + + + + + + + + + + + + + + + + + + + + + + + run + + + + + + + + + diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/Fuzzing.java b/sdk/src/test/java/io/opentdf/platform/sdk/Fuzzing.java new file mode 100644 index 00000000..776a107e --- /dev/null +++ b/sdk/src/test/java/io/opentdf/platform/sdk/Fuzzing.java @@ -0,0 +1,69 @@ +package io.opentdf.platform.sdk; + +import java.io.IOException; +import java.io.OutputStream; +import java.nio.ByteBuffer; +import java.security.NoSuchAlgorithmException; +import java.text.ParseException; + +import org.apache.commons.codec.DecoderException; +import org.apache.commons.compress.utils.SeekableInMemoryByteChannel; + +import com.code_intelligence.jazzer.api.FuzzedDataProvider; +import com.code_intelligence.jazzer.junit.FuzzTest; +import com.google.gson.JsonParseException; +import com.nimbusds.jose.JOSEException; + +import io.opentdf.platform.sdk.TDF.FailedToCreateGMAC; +import io.opentdf.platform.sdk.TDF.Reader; + +public class Fuzzing { + private static final String testDuration = "600s"; + private static final OutputStream ignoreOutputStream = new OutputStream() { + @Override + public void write(int b) { + // ignored + } + + @Override + public void write(byte b[], int off, int len) { + // ignored + } + }; + + @FuzzTest(maxDuration=testDuration) + public void fuzzNanoTDF(FuzzedDataProvider data) throws IOException { + byte[] fuzzBytes = data.consumeRemainingAsBytes(); + NanoTDF nanoTDF = new NanoTDF(); + nanoTDF.readNanoTDF(ByteBuffer.wrap(fuzzBytes), ignoreOutputStream, NanoTDFTest.kas); + } + + @FuzzTest(maxDuration=testDuration) + public void fuzzTDF(FuzzedDataProvider data) throws FailedToCreateGMAC, NoSuchAlgorithmException, IOException, JOSEException, ParseException, DecoderException { + byte[] fuzzBytes = data.consumeRemainingAsBytes(); + byte[] key = new byte[32]; // use consistent zero key for performance and so fuzz can relate to seed + var assertionVerificationKeys = new Config.AssertionVerificationKeys(); + assertionVerificationKeys.defaultKey = new AssertionConfig.AssertionKey(AssertionConfig.AssertionKeyAlg.HS256, key); + Config.TDFReaderConfig readerConfig = Config.newTDFReaderConfig( + Config.withAssertionVerificationKeys(assertionVerificationKeys)); + TDF tdf = new TDF(); + + try { + Reader reader = tdf.loadTDF(new SeekableInMemoryByteChannel(fuzzBytes), TDFTest.kas, readerConfig); + + reader.readPayload(ignoreOutputStream); + } catch (SDKException | InvalidZipException | JsonParseException | IOException | IllegalArgumentException e) { + // expected failure cases + } + } + + @FuzzTest(maxDuration=testDuration) + public void fuzzZipRead(FuzzedDataProvider data) { + byte[] fuzzBytes = data.consumeRemainingAsBytes(); + try { + ZipReaderTest.testReadingZipChannel(new SeekableInMemoryByteChannel(fuzzBytes), false); + } catch (InvalidZipException | IllegalArgumentException | JsonParseException | IOException e) { + // cases which are expected with invalid fuzzed inputs + } + } +} diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/NanoTDFTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/NanoTDFTest.java index 87bae33b..cd4c056d 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/NanoTDFTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/NanoTDFTest.java @@ -37,7 +37,7 @@ public class NanoTDFTest { private static final String KID = "r1"; - private static SDK.KAS kas = new SDK.KAS() { + protected static SDK.KAS kas = new SDK.KAS() { @Override public void close() throws Exception { } 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 5c47710c..29769b2b 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java @@ -12,9 +12,13 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.File; import java.io.IOException; import java.io.RandomAccessFile; +import java.nio.channels.SeekableByteChannel; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.HashMap; import java.util.Map; import java.util.Random; @@ -32,21 +36,31 @@ public class ZipReaderTest { public void testReadingExistingZip() throws Exception { try (RandomAccessFile raf = new RandomAccessFile("src/test/resources/sample.txt.tdf", "r")) { var fileChannel = raf.getChannel(); - var zipReader = new ZipReader(fileChannel); - var entries = zipReader.getEntries(); + ZipReaderTest.testReadingZipChannel(fileChannel, true); + } + } + + protected static void testReadingZipChannel(SeekableByteChannel fileChannel, boolean test) throws IOException { + var zipReader = new ZipReader(fileChannel); + var entries = zipReader.getEntries(); + if (test) { assertThat(entries.size()).isEqualTo(2); - for (var entry: entries) { - var stream = new ByteArrayOutputStream(); - if (entry.getName().endsWith(".json")) { - entry.getData().transferTo(stream); - var data = stream.toString(StandardCharsets.UTF_8); - var gson = new GsonBuilder() - .registerTypeAdapter(Manifest.class, new ManifestDeserializer()) - .create(); - var map = gson.fromJson(data, Map.class); - + } + for (var entry: entries) { + var stream = new ByteArrayOutputStream(); + if (entry.getName().endsWith(".json")) { + entry.getData().transferTo(stream); + var data = stream.toString(StandardCharsets.UTF_8); + var gson = new GsonBuilder() + .registerTypeAdapter(Manifest.class, new ManifestDeserializer()) + .create(); + var map = gson.fromJson(data, Map.class); + + if (test) { assertThat(map.get("encryptionInformation")).isNotNull(); } + } else if (!test) { + entry.getData().transferTo(stream); // still invoke getData logic } } } diff --git a/sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzNanoTDF/sample.ntdf b/sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzNanoTDF/sample.ntdf new file mode 100644 index 0000000000000000000000000000000000000000..7dd6dcbbb707ad930cfd8ac55f7c0e161df50c1a GIT binary patch literal 361 zcmV-v0hazuF-#E?VQ^_KWq4t2aBO8RV{dIQYhiP8F#rJq0S6vI?zpV-5A$o9H~hf> z5o5XkqDq=e5VP{lU_k`b@%q&1b&H|C`^^;qA@Zi}e!KLw)!YoehT+CBHGGG1UUe~n zSvv{ViMOExD^~BBlG(zv<-h4*CkILl)c>a*u#}LR^Xt{L?j_ZQMBO4o z)vl|6ccp}%F}}ZZR#}d4;LKH3a!&GhdH7X<>*u-7u0i{vv`*sh;W69If?W?SNq4*t zVT>)8y#Yqx=83kN5M+PM<`NAOOz9pgWt6#yP5# z0UU4s5$q$M6k!OYDH8*-haxv3wlfWW)*p38SE!kvyEwgJ?E&6C;9uMXzFO-c^8)w& zFoDm%b3rbh=(T-e6!3)V7w#;Fzhk!KTD9FpE{gyF7v$mPUYH8j2dV*CFAn)_Q!y-g H4Rd`h994Bd;xS8qn+CvX2SKE<<9=0=+0}=?mK(fFR(8&SAN-SC)g0(PoGW`m@ zrXMIDC6^xh5!zh@vc_$8M%v~7+TZ`R|M}#bZ!ej%GluzOd@>la{wde@z9?)F={O9D#3r!tLky0W6c<@&LkR3pYD}f`@ z?Pg`8ka7tM8WwXUW~R1H4bOz8=K99UbcyR7(^tINYD9QEr+AyrOuK!5gj!fA-Jx)a zA({|^^9boDldp>1+S za|wRCqpTLL>^YvHxybOgZQ_qV6A&ak&frAiAMsZZA5G!5HPhtIOc;2kw(t~RY{~x8 zn`w8ssSS{PlFC}`m`rkO6o)vpLqwyvwUU$IusDloOxQHGq6j#rtcSQza50YF)$1s< zE#!s?e%I}^TU`B_ol<`SGHT5kx8_b_>#O~AU-ur=?et2NUDQUKsoGnQZbT4DoQ=B* zP}BK#?$;rZ2i-UmZaoR?OpL=RdgOM!{xX8wdM^wmr#`@uA&&<2E32~@XycZ29=4}x zTF<3!KH4lfelE^u=TdYvTcANEE8}(0)x7JztEFJl+pTp`pF&q#b`#AU;AG`F%bTv* zp6K^eaE+yCtM{TiK2eeH*~0jSYwY7)5Vf2==A?A(^Kh{;`m+8Q?C!+=PU&oyx;)NO zdq1@Bo-@ZPUx$yLFpJ^ixf~J`-J3~08}Q95LEo&sbw_Wj(@bsX8_Try#NqqB`}1(q zL-Vw|=kI$lpY2ACC2abMyf73&Js)VOnXN<}HHT4__0yA1ElTynp-osHU^jG5ijWpy zNFFTLr^(<&^^!a&mr`Fork90@ieq5;);*&6$MFNGnvR8gs)oZH1lP^s6q{SD%;twz zqH2#r{Zl!Rjp04F=#0GRYUD3mM;i2;;UcqECca8@Y{Vl9-w#BBs!D$!nJ>{?e5>1FmmLEK2XI3Qi&f zF$GybWXp*`5LT~vOFTUDq{>enW_{=bt!x9*1f;Kv_rD(67~spK2vc?0X(#0{s>l_F z7_31|rU|GRWe*((Sb^`&$NA*N%S+~I;L$PmH1K|=v#VT`denZw{B)dh)wHVwE0at) zL!ZyRN{(Tsmpg7#g(t^A=o G!~6?Y?=yV> literal 0 HcmV?d00001 diff --git a/sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzTDF/crash-InvalidManifest-2 b/sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzTDF/crash-InvalidManifest-2 new file mode 100644 index 0000000000000000000000000000000000000000..de7914cd21fc577ab43fae901c0934eb6431f1d1 GIT binary patch literal 2017 zcmZ`)$&#B!7{*RwPh6GCHHRD&uCqu&%gngS2P6>M0F7WIAmsxCEzzi11U=F~%9U5h z)p;O4iZ40j5t42M8kG}w73x<1OMm}9n|$-_C3AMhFdxm2CPUUg<@%nS8&|QNp~!ZJ z0HNrjwvQ6z{xX}qKKuOX^ItzucCxB{R_LNPRM(%M^ug{G!_e;ckhl+3k{_?6fRLj z143{fA$=%M?~A>30=oej?>Hbmg@4G0LS-Bw=wv+@x-g`{c_tCNz7j`5scbRzb*|_h z!S8gG<=m5f*Ee+!nf|6t>K|V+*N?K zpKWGA9RhjKO)}xmm%!G-IGUm-ZrkfGVz{aIqDXS<100+3Xi&elJM)1)Zb|1+d%EA( zbE%t;)(eiGi8J+Fim%l<8f3CEUWHxVzv+AW9!z@Ml_45a=;@1Ys#^n`E`4`#+qK#g z<6#PJuoQ2MUVP7|8VYM2G1}TR(;CLwwj3jM`F`)=JX-hA zY~S7S550uXwxh-ZHiJ~2n~I>F4|LSbmZE{0!#K;QK)&?u;?9e5u z@;Kx_N!X^bGI zAPb0WIWY*rs*AV8Lz^dAerhlqzyRoF8IU3%bzSuTdgNe$FVn(J)nliXl*72Xt}rBE z1rjn%Kt(8P=s3U%d~ZI?CNExIGS35#jD27|9?hH3uOzs6}hB)CP-`E3c5M zoF)%6r##18a>yej-9o~|6<^a;qMQD=@89|#55D^Pj5$7Lm`}zhgCXnhe0@vKmGj6- zQD`|`fKYf^+JrH3K8y$Nj=y~V^5=JyovdOXXS(PO)%E*lt-XHBF!cML+*QtfD}W1- z;BzktgP-4*N-PNMIE^q2gI=%*W8b1j_PRuggbV`=mNCTXe^H;sfun+@4+5;hEfB%N zdO(;csaEQjjY|DA(?z)BFeDtyz`_qP*c-_VS!hWv*sjX)4Ym*`zM`!}4cuRhy-QMh>W*TyL zXkLgSnh=6>59ujCea!Ye2G?^cXZO_yup5jY2 z*`InN?QUyoZ6qJ0vQ*k9lUR-75Qlb%XcSjgau6IArxA?_yNRtR0?r}nA9m_pcamDOe%0wr zx8#1Vs~c!`5Gm*>&i?C_q!&!9*6bFJU>_=)T$4qF6@Ti=oBFmlEz1>S6F+c35Truqm#zG~N&(MF*y`zGS>_hUsn1bWM+qb7V3&mBeH~FF1f6f2@ dMTRL|MD88kf60CshME}2<(*o3if(NFeGjS>kN1{iE(h_nBrK8gcJ1zR5kScN$dz{a{q zm?&vB+E?91`&FTfaK~XtIJSX>?_+Q>QW&z(mK?AnmFslbN`QzYusByZqiK2*WUR%u z!il-!*_@KwEhXPRDAFxwN@7#-$8AMWTvHo&%(c-`1e=>nLLZr$ zLF8_7g@5~&Eup=yOtQ(*k&%_S{1KBR^MezlivC|mJH77bN)qDI&!vT08B*%)7XnAP z+s#Tx!R7vuxHK&0TFgvsml~c4P0jU@<=Eyi755O z(6+gvxdgx6QPxXW_8ia9Tx58=Hu1-w3kVV#XK*6%kN7Ky52kR_nrm`rE(|?WTY8Ew zwq$?h&9&Ry)P_htOJ%KgN+z)y#UT#u5K+rJD>(}ei?fKvgiT{Bihy%Yx{nJ17vtzn zy^ccLLT;GgH!ZH$?AAZiQ_4?4MzuMU*1}0_eSMf6>fXJ&oApH5MQya5sr}9PS_GlQ z*|@6!HC^l$ejNgN*o`yc#*@I_#5kOx2X5aVtRlFp_rp+f>O&kE@_1P9S)Jujo3x~h zusuuDdM8{O*C_eleOop zuDfP?s^87P6_%o%-j8niL`A-53zKWEaftUp)N&4(lhTdP!{yo-$ofODzZD02rL$Y< z@+3>`!^px1&YY-x9X@!%JcbV!a!6Ekuc!5F$Txd}zTJ46j^0*hncC2|mTBvW!w>p* z7vUE57issv-}PfY+m9P7*z^;5X()nvG1O2qTZ=ksj-o6Zq-UL4lZs?G> zS4Bt*FeDe2>(gZLqIyX#luKzKpVG_1M8z?%eCrO;{Nwl@R87ai0~Nzz4uY%ZXok%l zR_2T2Em5_{vHmF^$R_ZPTXx1?)EoOt*O7*OXSB?$^`z-YtDKzQ>1qm136X+c;o@I! zB;#P!wAMFp4ku920nYGyJ0wxCx2(4Ara0lgqQ0RR91 literal 0 HcmV?d00001 diff --git a/sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzTDF/crash-InvalidManifest-NullKeyAccessObj b/sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzTDF/crash-InvalidManifest-NullKeyAccessObj new file mode 100644 index 0000000000000000000000000000000000000000..d55d907b34a1658970daf36f31943291eeb7b40b GIT binary patch literal 2017 zcmZ`)$&#B!7{*RwJFZIQnnMmURXNR!C81@ODj$$QXah8Yk${vB475a}W)bvA11VQt zAy?;t{3yQUkViJ`I$G(VaQS^t#ldvb2PjqMCY zwmSp}Md!7Blpy!bZ1VEe=TD#i`hkj*RqeCF7Nt;Ie}2*jyB7>Ye?Jmh<85Gva0OEQ z)=#7Gx7W2A3qmK!Vhp2j6t1Equ<4P#sZl0j!w`dY0&(_#+%M|D)xbIcA=Y3H1hBFn z5GF>NjrLWy(LOJ15$Pm~h{QUy@k0WRmlPM-XiW(0P~$pXwo)J}DJ@@>Pj{}I5Mx~)JRHP`n9~FR)&;%`<1{E z>2|ZSQAl|N1r3X}6f;ZT>`mW-mhJ`S((;Jw9V<|LbvYtDo>TmFN7ZlN9ibK$N_Qw+ zVu&V$;5|F12W!mKzf9KehQwABLtnS2SXQzG&zS-p|=sgzLH16seCc@ zb*|_h!S8gG<=m5f*Ee+!nf|6t{0US6LBiuqP9**je+BW;5^h_nE_YO6;9L6KR|2sm z2Mb@-?{Z5YAo(Pfwc0V6#?AGFQ%Qj|T^M(e58TaB(o5J{Ya zy9&_uv&}51Lm&^jNhaL-64+W8M^p63ZF~Ji3^(;&6iIGL>dVLUvM|+f0_?!PM>PL9c>q<@v2aJ#aFm1asyUouYl9Va zc6cSK_Bhl(l>^xr-gEQL$d4~a!Q6ADLC+n|GkZC1y3!&i^m{|wLrX%WpclCKHX7+D zTr}XyW!^uDU# zG)53pkOf4xoEQXQ^@_K|!!u8+{M2DKfC13UHXuzv`nq`k>yd*2zDRNMiyc&#opw?V zFg>Or5?4PF+UxrTs7?~ z!OA34&d{fGuaaY!DLFowg0sns7w4q}#Z|56<)PaDD8K(E!;~(f_mb|v7yk_Nte|~Y La4Uc4&M^N1MAtJ( literal 0 HcmV?d00001 diff --git a/sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzTDF/crash-InvalidManifest-NullSegment b/sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzTDF/crash-InvalidManifest-NullSegment new file mode 100644 index 0000000000000000000000000000000000000000..4537beda55a232f533c1c62532548e2c2f789d19 GIT binary patch literal 2017 zcmZ`)$&#B!7{*RwJFZIQnnMl>*YQ{qT4r(ifCNHF0%-&z0Vy9CXo*J6BIuC@Qm(v0 zuFeDbQGCfEkC1dD&?u9ntE6uJt-pWke>VQ=>vQJSD~9=Geli)d{wmkE% zcL)%Q-qiL{g4~~G<2lt|cc%Ky`y^r6_xAg~*d@s0!15`KaBP!EO4I6~0LdN6chNR#u(B8dMd zk0-vE`Z`y2kKlJY>T>QWzU!O1hfIIdCjJCd0YSp!Oim>J5q|~o!4hs-Q(fsyg@JGB zb6*X_mJ%%dseYSV`T!{>sU%)K#*mGZ2uDtYXcRYgdJ-HKXEBWlyHD&m2JR{AJ}DGD zjN|Ki9Yv0fyeP%j-A=p3)t}fY^~WHi*4$BR=BAFZ+)Z|M|6bcnE=0vcZM2?fy_I|= zf=J>V+*N_LpKWGA9Rg+0O)}xem%!G-IGUgbZrkfGVz{aIqDXS<100)*Jg8sTo%ui? zwWPDCJ=yQ;xzx?&^@8JP;%s^*#TU~#8f1z(T7_NRzwCSZ9*le2l_44v=;@1Ys#^n` zE`4`#)wSAV<8A^ju@rBNUVO`^8VYsIc-j8&4m5Wl3+RvCDj#I9h zc9mddk|}5C)45m4G0cP@*T+J7s*|0KhdE~596?!OlQ4D+m@ NeN%8Nf9TFIe*;(}GuQwC literal 0 HcmV?d00001 diff --git a/sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzTDF/sample.tdf b/sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzTDF/sample.tdf new file mode 100644 index 0000000000000000000000000000000000000000..cc27081a3c3c30eb85308bda48c3e8c15736604f GIT binary patch literal 2017 zcmZ`)OLLn>94Bd;xS8qn+CvX2SKE<<9=0=+0}=?mK(fFR(8&SAN-SC)g0(PoGW`m@ zrXMIDC6^xh5!zh@vc_$8M%v~7+TZ`R|M}#bZ!ej%GluzOd@>la{wde@z9?)F={O9D#3r!tLky0W6c<@&LkR3pYD}f`@ z?Pg`8ka7tM8WwXUW~R1H4bOz8=K99UbcyR7(^tINYD9QEr+AyrOuK!5gj!fA-Jx)a zA({|^^9boDldp>1+S za|wRCqpTLL>^YvHxybOgZQ_qV6A&ak&frAiAMsZZA5G!5HPhtIOc;2kw(t~RY{~x8 zn`w8ssSS{PlFC}`m`rkO6o)vpLqwyvwUU$IusDloOxQHGq6j#rtcSQza50YF)$1s< zE#!s?e%I}^TU`B_ol<`SGHT5kx8_b_>#O~AU-ur=?et2NUDQUKsoGnQZbT4DoQ=B* zP}BK#?$;rZ2i-UmZaoR?OpL=RdgOM!{xX8wdM^wmr#`@uA&&<2E32~@XycZ29=4}x zTF<3!KH4lfelE^u=TdYvTcANEE8}(0)x7JztEFJl+pTp`pF&q#b`#AU;AG`F%bTv* zp6K^eaE+yCtM{TiK2eeH*~0jSYwY7)5Vf2==A?A(^Kh{;`m+8Q?C!+=PU&oyx;)NO zdq1@Bo-@ZPUx$yLFpJ^ixf~J`-J3~08}Q95LEo&sbw_Wj(@bsX8_Try#NqqB`}1(q zL-Vw|=kI$lpY2ACC2abMyf73&Js)VOnXN<}HHT4__0yA1ElTynp-osHU^jG5ijWpy zNFFTLr^(<&^^!a&mr`Fork90@ieq5;);*&6$MFNGnvR8gs)oZH1lP^s6q{SD%;twz zqH2#r{Zl!Rjp04F=#0GRYUD3mM;i2;;UcqECca8@Y{Vl9-w#BBs!D$!nJ>{?e5>1FmmLEK2XI3Qi&f zF$GybWXp*`5LT~vA|9T3Qst)(vp)2JR<;3Y0@ByT`(F=j4De-AgsHmhw3BieRpbgo z4Avkf(*#tEvWJcXtibo?<9zbs+i|jgaoMfDr^YEOu@Y<7j?6Ey(PagKN z1vL^4zW@qaYDy$TLBWsU5727ezWLtsy+-|apB@!n6bgllq_gDL{d%86 zKYyl%-Pet;J{a|%zes+6 z{)xcF@-sRp&b8%=fBWH2`;TXZLViBtxbkseg?NP$cI+ilcqLd#f?|fFFcF$BFNy*R z?Kq`5LJL<>99TIhG_M34GPI5{OW(lX3Ls~S)&UCH6lX|5D{DiT&^(c-aQ@Q8*?y?_I}m_OH|&hVDe$xm80(6z5ZClHSvTe$ zR6zxLE`jQJ4}u;BMr$`TW>v75Nzfn!E_%Kmj5#lH;rjGcxaJmayz5$t+d7z5LF+uX z{1&e(Ww~BGsofLv&|KX`=FvxmrX&{ymn-G&LvP{%!_Rbayyss@fiiKS*Oqm~7NIeg zp~t^u=pInpr;jZQ&D!+^chW~`%eGN+V~|D!+vyn&9USK2fyyj}UJvW)xXeQ&nVv1d zz}!PcD+7*$hA-+1NX*4N01MrPL0f{}SOkj>HpOujWHS+XpsK4qf2g$ho9{e4MS;de z7TFQuR(vbr2$oWwCSey_lp=>CHgQgL8KccoiA1(V+$dqqT2+?Bl0|XpM($Rik5bMZ z?{saivfVksC$Q(Q%9L&!BEa-ZqV>=E^%l(*)P6N{2OaXNC(r7MSz5wqqp%rGktESx zcY9uGPl+WvRWR8{R6g4Vz%%A*XIl@L;W z#uOWDAa=@5SKE%oy=zN4g#q!*<%C{}^41ADiL;n(jj5Js!GzkxSxmMFWUp50xHjVXuVDREGY^JO23{1=NkSBh?3kw9} zbhsExJfW_c_8Jd)N+`iBFZ0{6acJ12ebdW4e@zx|P*DhQfQ*~W^1SluI&^m_vKeAW z35$^x9Eu^jD~1fy2O}|Bp_sQjx-Wp{%q;2Yy>)PMF~&hUh9#_!SL^&jZ6t_c7D literal 0 HcmV?d00001 diff --git a/sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzZipRead/crash-f39ad8416aef7cf275f84683aaa0efd15f24272a b/sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzZipRead/crash-f39ad8416aef7cf275f84683aaa0efd15f24272a new file mode 100644 index 0000000000000000000000000000000000000000..2fc1da228a64d5718fd2608a080af9c93529bc53 GIT binary patch literal 99 gcmWH@D$dUf@MdLW;0*9)XJBF|VW1ueU;~*20A$VtC;$Ke literal 0 HcmV?d00001 diff --git a/sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzZipRead/sample.txt.tdf b/sdk/src/test/resources/io/opentdf/platform/sdk/FuzzingInputs/fuzzZipRead/sample.txt.tdf new file mode 100644 index 0000000000000000000000000000000000000000..2bb81265469ccc3b2065a5bb778e88aa3f13fa65 GIT binary patch literal 1754 zcmZ`)Kab-?6kp(uI}j2=LW0@_658Ec$9A%th$}X6;y;O!|+oO*eYF2-fz2n+j?fBR4|L}f)cyv^MA5tFsBz6}8DV>||#$vS9Vy)6mucGTx3zbSUHw|4x zfgLJ>ZZ1Mc(i%<}AV)HGTC76Nyxfk>c~i6IykjEdSZbul2GvD@vw8ZIxuFpbyOZ5K zxZH=9w98%{`G0oZJa63z>(V@bS((=oWjZ`fE}%~7_MLDVYGzdFoUx;+l$tmVoUkwG zqQ^O=AvhsT$<#wl>OXxwGSg|@JfoY8KzZqTAU_1eDS}?vg^3T7x;&0JFP%_@x@54l zM7$k(yc63yM^spi@*Febbjv}uwTLxK51hEqJE6g8)&Sa^(bTFrr-fQmm(*x1UQ(LA zb$L!0oTVi7QbeOfZcZ7b#Ppb$ZRTPOe9E}aYGFVK?wp;W)N@gg=A_eV3Osk_Vt972 zyEPbsl4@sLUD-)w+e7f&QKL1Bana^9h>wvn><%w3ab;oeY95RR=t>pl7rA}5bkb{) z%y9{L9;)BE!8ZJ zvz!et%yU^c+J==Pr6KR{GX-i4NRaxsP)aEmRO^J|10t{WACQiUsS+EhY(uEf*8mj+#5o|dkqHhgJO zQT~eR1oyl-978pw^wz7=V#F;3+iGlA()`-y+GNBHSYK8OgWCljuVW`hp}m~q6(<4&6*f1>U8!C9V#?{J zQE9Lpcv4i<`n8=WFW`(@&^)N0q~VWxYUD2U%CT5DM#Nvww$NWLu+vjoN*D=ttO&Nb z*^Nw!UDG4F)UUJ|D~u`bMikdtY?Ncm;3=L0yHwlvFv&53WM9KW*WUNbynkq=6h615 zk6{eV1F$HpD7g2{^(^%WAgB5Mq^QqVuvkHSA0z{7kkM`nDC>a6yD7ok+)RdVy?uIg rKT%#C3H3yI{kgMzX;zE>YhK)&*>w5F^q34EzFRB2OxBM>{ literal 0 HcmV?d00001 From 786c557bf2ef4e63ba9fee8eb580ac30c73bcf5f Mon Sep 17 00:00:00 2001 From: Mike Jensen Date: Wed, 27 Nov 2024 12:17:34 -0700 Subject: [PATCH 8/9] Fix test from minimum segment size enforcement This change was applied easily in go, but there are issues with integration and other existing test payloads. Because this is low risk, I believe it's ok to remove this protection in the Java SDK, but leave commented so it's known to be explict. Alternatively we could update test payloads. --- .../main/java/io/opentdf/platform/sdk/Config.java | 2 +- .../main/java/io/opentdf/platform/sdk/TDF.java | 4 ++-- .../java/io/opentdf/platform/sdk/ConfigTest.java | 15 +++++++++++++-- .../java/io/opentdf/platform/sdk/TDFTest.java | 9 ++++----- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/Config.java b/sdk/src/main/java/io/opentdf/platform/sdk/Config.java index 8f1e7da3..a8b58c4b 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/Config.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/Config.java @@ -22,7 +22,7 @@ public class Config { public static final int TDF3_KEY_SIZE = 2048; public static final int DEFAULT_SEGMENT_SIZE = 2 * 1024 * 1024; // 2mb public static final int MAX_SEGMENT_SIZE = DEFAULT_SEGMENT_SIZE * 2; - public static final int MIN_SEGMENT_SIZE = 16 * 1024; + public static final int MIN_SEGMENT_SIZE = 16 * 1024; // not currently enforced in parsing due to existing payloads in testing public static final String KAS_PUBLIC_KEY_PATH = "/kas_public_key"; public static final String DEFAULT_MIME_TYPE = "application/octet-stream"; public static final int MAX_COLLECTION_ITERATION = (1 << 24) - 1; 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 c24628e2..ca01fd21 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java @@ -361,9 +361,9 @@ public void readPayload(OutputStream outputStream) throws TDFReadFailed, for (Manifest.Segment segment : manifest.encryptionInformation.integrityInformation.segments) { if (segment.encryptedSegmentSize > Config.MAX_SEGMENT_SIZE) { throw new IllegalStateException("Segment size " + segment.encryptedSegmentSize + " exceeded limit " + Config.MAX_SEGMENT_SIZE); - } else if (segment.encryptedSegmentSize < Config.MIN_SEGMENT_SIZE) { + }/* else if (segment.encryptedSegmentSize < Config.MIN_SEGMENT_SIZE) { throw new IllegalStateException("Segment size " + segment.encryptedSegmentSize + " is under minimum " + Config.MIN_SEGMENT_SIZE); - } + }*/ // Commented out due to tests needing small segment sizes with existing payloads byte[] readBuf = new byte[(int) segment.encryptedSegmentSize]; int bytesRead = tdfReader.readPayloadBytes(readBuf); diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/ConfigTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/ConfigTest.java index cae23434..1754428d 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/ConfigTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/ConfigTest.java @@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; class ConfigTest { @@ -46,8 +47,18 @@ void withMetaData_shouldSetMetaData() { @Test void withSegmentSize_shouldSetSegmentSize() { - Config.TDFConfig config = Config.newTDFConfig(Config.withSegmentSize(1024)); - assertEquals(1024, config.defaultSegmentSize); + Config.TDFConfig config = Config.newTDFConfig(Config.withSegmentSize(Config.MIN_SEGMENT_SIZE)); + assertEquals(Config.MIN_SEGMENT_SIZE, config.defaultSegmentSize); + } + + @Test + void withSegmentSize_shouldIgnoreSegmentSize() { + try { + Config.newTDFConfig(Config.withSegmentSize(1024)); + fail("Expected exception"); + } catch (IllegalArgumentException e) { + // expected + } } @Test 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 35416f37..96f5b980 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java @@ -359,11 +359,10 @@ public void testCreatingTDFWithMultipleSegments() throws Exception { Config.TDFConfig config = Config.newTDFConfig( Config.withAutoconfigure(false), Config.withKasInformation(getKASInfos()), - // use a random segment size that makes sure that we will use multiple segments - Config.withSegmentSize(1 + random.nextInt(20))); + Config.withSegmentSize(Config.MIN_SEGMENT_SIZE)); - // data should be bigger than the largest segment - var data = new byte[21 + random.nextInt(2048)]; + // data should be large enough to have multiple complete and a partial segment + var data = new byte[(int)(Config.MIN_SEGMENT_SIZE * 2.8)]; random.nextBytes(data); var plainTextInputStream = new ByteArrayInputStream(data); var tdfOutputStream = new ByteArrayOutputStream(); @@ -418,7 +417,7 @@ public void write(byte[] b, int off, int len) { var tdfConfig = Config.newTDFConfig( Config.withAutoconfigure(false), Config.withKasInformation(getKASInfos()), - Config.withSegmentSize(1 + random.nextInt(128))); + Config.withSegmentSize(Config.MIN_SEGMENT_SIZE)); assertThrows(TDF.DataSizeNotSupported.class, () -> tdf.createTDF(is, os, tdfConfig, kas, null), "didn't throw an exception when we created TDF that was too large"); From 90aa02f617086ce59addf16dcad0de0bef047f62 Mon Sep 17 00:00:00 2001 From: Mike Jensen Date: Wed, 27 Nov 2024 12:33:15 -0700 Subject: [PATCH 9/9] Style fixes from Sonar CI --- .../java/io/opentdf/platform/sdk/Manifest.java | 2 -- .../main/java/io/opentdf/platform/sdk/TDF.java | 4 +--- .../java/io/opentdf/platform/sdk/ConfigTest.java | 2 +- .../java/io/opentdf/platform/sdk/Fuzzing.java | 16 ++++++++-------- .../java/io/opentdf/platform/sdk/TDFTest.java | 5 ----- .../io/opentdf/platform/sdk/ZipReaderTest.java | 4 ---- 6 files changed, 10 insertions(+), 23 deletions(-) diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/Manifest.java b/sdk/src/main/java/io/opentdf/platform/sdk/Manifest.java index b72fad64..3f7fd8e8 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/Manifest.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/Manifest.java @@ -483,8 +483,6 @@ protected static Manifest readManifest(Reader reader) { throw new IllegalArgumentException("Manifest with null encryptionInformation"); } else if (result.encryptionInformation.integrityInformation == null) { throw new IllegalArgumentException("Manifest with null integrityInformation"); - } else if (result.encryptionInformation.integrityInformation.segments == null) { - throw new IllegalArgumentException("Manifest with invalid integrityInformation"); } else if (result.encryptionInformation.integrityInformation.rootSignature == null) { throw new IllegalArgumentException("Manifest with null rootSignature"); } else if (result.encryptionInformation.integrityInformation.rootSignature.algorithm == null 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 ca01fd21..4bbd8008 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java @@ -361,9 +361,7 @@ public void readPayload(OutputStream outputStream) throws TDFReadFailed, for (Manifest.Segment segment : manifest.encryptionInformation.integrityInformation.segments) { if (segment.encryptedSegmentSize > Config.MAX_SEGMENT_SIZE) { throw new IllegalStateException("Segment size " + segment.encryptedSegmentSize + " exceeded limit " + Config.MAX_SEGMENT_SIZE); - }/* else if (segment.encryptedSegmentSize < Config.MIN_SEGMENT_SIZE) { - throw new IllegalStateException("Segment size " + segment.encryptedSegmentSize + " is under minimum " + Config.MIN_SEGMENT_SIZE); - }*/ // Commented out due to tests needing small segment sizes with existing payloads + } // MIN_SEGMENT_SIZE NOT validated out due to tests needing small segment sizes with existing payloads byte[] readBuf = new byte[(int) segment.encryptedSegmentSize]; int bytesRead = tdfReader.readPayloadBytes(readBuf); diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/ConfigTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/ConfigTest.java index 1754428d..b3bc3f77 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/ConfigTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/ConfigTest.java @@ -54,7 +54,7 @@ void withSegmentSize_shouldSetSegmentSize() { @Test void withSegmentSize_shouldIgnoreSegmentSize() { try { - Config.newTDFConfig(Config.withSegmentSize(1024)); + Config.withSegmentSize(1024); fail("Expected exception"); } catch (IllegalArgumentException e) { // expected diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/Fuzzing.java b/sdk/src/test/java/io/opentdf/platform/sdk/Fuzzing.java index 776a107e..5fcb90fc 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/Fuzzing.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/Fuzzing.java @@ -18,27 +18,27 @@ import io.opentdf.platform.sdk.TDF.Reader; public class Fuzzing { - private static final String testDuration = "600s"; - private static final OutputStream ignoreOutputStream = new OutputStream() { + private static final String TEST_DURATION = "600s"; + private static final OutputStream IGNORE_OUTPUT_STREAM = new OutputStream() { @Override public void write(int b) { // ignored } @Override - public void write(byte b[], int off, int len) { + public void write(byte[] b, int off, int len) { // ignored } }; - @FuzzTest(maxDuration=testDuration) + @FuzzTest(maxDuration=TEST_DURATION) public void fuzzNanoTDF(FuzzedDataProvider data) throws IOException { byte[] fuzzBytes = data.consumeRemainingAsBytes(); NanoTDF nanoTDF = new NanoTDF(); - nanoTDF.readNanoTDF(ByteBuffer.wrap(fuzzBytes), ignoreOutputStream, NanoTDFTest.kas); + nanoTDF.readNanoTDF(ByteBuffer.wrap(fuzzBytes), IGNORE_OUTPUT_STREAM, NanoTDFTest.kas); } - @FuzzTest(maxDuration=testDuration) + @FuzzTest(maxDuration=TEST_DURATION) public void fuzzTDF(FuzzedDataProvider data) throws FailedToCreateGMAC, NoSuchAlgorithmException, IOException, JOSEException, ParseException, DecoderException { byte[] fuzzBytes = data.consumeRemainingAsBytes(); byte[] key = new byte[32]; // use consistent zero key for performance and so fuzz can relate to seed @@ -51,13 +51,13 @@ public void fuzzTDF(FuzzedDataProvider data) throws FailedToCreateGMAC, NoSuchAl try { Reader reader = tdf.loadTDF(new SeekableInMemoryByteChannel(fuzzBytes), TDFTest.kas, readerConfig); - reader.readPayload(ignoreOutputStream); + reader.readPayload(IGNORE_OUTPUT_STREAM); } catch (SDKException | InvalidZipException | JsonParseException | IOException | IllegalArgumentException e) { // expected failure cases } } - @FuzzTest(maxDuration=testDuration) + @FuzzTest(maxDuration=TEST_DURATION) public void fuzzZipRead(FuzzedDataProvider data) { byte[] fuzzBytes = data.consumeRemainingAsBytes(); try { 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 96f5b980..326809f3 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java @@ -1,11 +1,6 @@ package io.opentdf.platform.sdk; -import com.google.common.util.concurrent.ListenableFuture; - import com.nimbusds.jose.JOSEException; -import io.opentdf.platform.policy.attributes.GetAttributeValuesByFqnsRequest; -import io.opentdf.platform.policy.attributes.GetAttributeValuesByFqnsResponse; -import io.opentdf.platform.policy.attributes.AttributesServiceGrpc; import io.opentdf.platform.sdk.Config.KASInfo; import io.opentdf.platform.sdk.TDF.Reader; import io.opentdf.platform.sdk.nanotdf.NanoTDFType; 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 29769b2b..f9819193 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java @@ -1,5 +1,4 @@ package io.opentdf.platform.sdk; -import com.google.gson.Gson; import com.google.gson.GsonBuilder; import io.opentdf.platform.sdk.Manifest.ManifestDeserializer; @@ -12,13 +11,10 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; -import java.io.File; import java.io.IOException; import java.io.RandomAccessFile; import java.nio.channels.SeekableByteChannel; import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.nio.file.Path; import java.util.HashMap; import java.util.Map; import java.util.Random;