Skip to content

Commit

Permalink
fix(sdk): Fuzz testing and protocol fixes (#214)
Browse files Browse the repository at this point in the history 
This change includes a variety of fixes found with fuzz testing.

* Protocol Exception improvements - `NullPointerExceptions` have been
converted to other exception types. Please provde feedback on if other
exception types should be used for these cases. In `Fuzzing.java` now
also serves to define in testing what exception types are acceptable.
The `catch` specifically lists the types of exceptions that were
discovered for each API call, and `throws` are checked exceptions that
are not expected to be possible. As a future improvement we may want to
refine this list further and better document what exceptions happen
under what conditions. For now I thought it was best to start with just
the `NullPointerException` cases. Since these cases were numerous, these
changes span multiple commits, with each commit focused on a specific
area of the protocol.
* Protocol DoS Fixes - The only memory consumption issue discovered was
the [counterpart found in the go
sdk](opentdf/platform#1536). A matching fix with
the same defaults was implemented here in the java sdk.
* Finally the testing itself is added as `Fuzzing.java` executed through
`sdk/fuzz.sh`. This script is long running, and there are occasional
Jazzer failures which are not believed to be real deficiencies (timeouts
when `.position()` is called on the stream). For that reason this
testing needs to be done manually, and not expected to be included in CI
* A few optimizations and clarity improvements were also included, as
they were noticed while generally trying to get familiar with the
codebase.
  • Loading branch information
@jentfoo
jentfoo authored Dec 4, 2024
1 parent cb6f757 commit cf6f932
Show file tree
Hide file tree
Showing 25 changed files with 341 additions and 121 deletions.
11 changes: 11 additions & 0 deletions sdk/fuzz.sh
View file
Original file line number Diff line number Diff line change
@@ -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
124 changes: 123 additions & 1 deletion sdk/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
<version>0.7.5</version>
</parent>
<packaging>jar</packaging>
<properties>
<jazzer.version>0.22.1</jazzer.version>
<jazzer.baseurl>https://github.com/CodeIntelligenceTesting/jazzer/releases/download/v${jazzer.version}</jazzer.baseurl>
</properties>
<dependencies>
<!-- Logging Dependencies -->
<dependency>
Expand Down Expand Up @@ -121,6 +125,18 @@
<version>4.13.2</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.code-intelligence</groupId>
<artifactId>jazzer-api</artifactId>
<version>${jazzer.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.code-intelligence</groupId>
<artifactId>jazzer-junit</artifactId>
<version>${jazzer.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-compress</artifactId>
Expand Down Expand Up @@ -307,4 +323,110 @@
</plugin>
</plugins>
</build>
</project>
<!--profile to execute fuzz test -->
<profiles>
<profile>
<id>fuzz</id>
<activation>
<activeByDefault>false</activeByDefault>
</activation>
<properties>
<skipTests>true</skipTests>
</properties>
<build>
<plugins>
<!-- Plugin to download and unpack the Jazzer agent -->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-antrun-plugin</artifactId>
<version>3.1.0</version>
<executions>
<execution>
<id>download-and-unpack-jazzer</id>
<phase>process-test-classes</phase>
<configuration>
<target>
<condition property="jazzer.os" value="windows">
<os family="windows"/>
</condition>
<condition property="jazzer.os" value="macos">
<os family="mac"/>
</condition>
<condition property="jazzer.os" value="linux">
<os family="unix"/>
</condition>
<echo message="Detected OS: ${jazzer.os}"/>
<mkdir dir="${project.build.directory}/jazzer"/>
<get src="${jazzer.baseurl}/jazzer-${jazzer.os}.tar.gz" dest="${project.build.directory}/jazzer/jazzer.tar.gz"/>
<untar compression="gzip" src="${project.build.directory}/jazzer/jazzer.tar.gz" dest="${project.build.directory}/jazzer"/>
</target>
</configuration>
<goals>
<goal>run</goal>
</goals>
</execution>
</executions>
</plugin>

<!-- Copy dependencies to a directory for Jazzer to reference -->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
<version>3.4.0</version>
<executions>
<execution>
<id>copy-dependencies</id>
<phase>process-test-classes</phase>
<goals>
<goal>copy-dependencies</goal>
</goals>
<configuration>
<outputDirectory>${project.build.directory}/dependency-jars</outputDirectory>
<includeScope>test</includeScope>
</configuration>
</execution>
</executions>
</plugin>

<!-- Plugin to execute Jazzer -->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-antrun-plugin</artifactId>
<version>3.1.0</version>
<executions>
<execution>
<id>run-jazzer-fuzzing</id>
<phase>verify</phase>
<configuration>
<target>
<path id="project.classpath">
<pathelement location="${project.build.directory}/classes"/>
<pathelement location="${project.build.directory}/test-classes"/>
<fileset dir="${project.build.directory}/dependency-jars">
<include name="**/*.jar"/>
</fileset>
</path>
<pathconvert property="project.classpath.string" pathsep="${path.separator}">
<path refid="project.classpath"/>
</pathconvert>
<property environment="env"/>

<chmod file="${project.build.directory}/jazzer/jazzer" perm="777"/>

<exec executable="bash">
<arg value="-c"/>
<arg value="if [ -z &quot;${JAVA_HOME}&quot; ]; then JAVA_HOME=$(dirname $(dirname $(which java))); fi; DYLD_LIBRARY_PATH=$(find &quot;${JAVA_HOME}&quot; -type d | grep 'libexec/openjdk.jdk/Contents/Home/lib/server' 2&gt;/dev/null | head -n 1); if [ -z &quot;${DYLD_LIBRARY_PATH}&quot; ]; then DYLD_LIBRARY_PATH=&quot;${JAVA_HOME}/lib/server&quot;; fi; export DYLD_LIBRARY_PATH=${DYLD_LIBRARY_PATH}; ${project.build.directory}/jazzer/jazzer --cp='${project.classpath.string}' --target_class='io.opentdf.platform.sdk.Fuzzing' --instrumentation_includes='io.opentdf.platform.sdk.**' ${jazzer.testDir}"/>
</exec>
</target>
</configuration>
<goals>
<goal>run</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
</profiles>
</project>
8 changes: 8 additions & 0 deletions sdk/src/main/java/io/opentdf/platform/sdk/Config.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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; // 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;
Expand Down Expand Up @@ -228,6 +230,12 @@ public static Consumer<TDFConfig> withMetaData(String metaData) {
}

public static Consumer<TDFConfig> 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;
}

Expand Down
39 changes: 36 additions & 3 deletions sdk/src/main/java/io/opentdf/platform/sdk/Manifest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -473,8 +473,41 @@ 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.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) {
Expand Down
2 changes: 1 addition & 1 deletion sdk/src/main/java/io/opentdf/platform/sdk/NanoTDF.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ PolicyObject createPolicyObject(List<String> 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) {
Expand Down
2 changes: 1 addition & 1 deletion sdk/src/main/java/io/opentdf/platform/sdk/SDKBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<InputStream> certStreams = new ArrayList<>();
List<InputStream> certStreams = new ArrayList<>(certFiles.length);
for (File certFile : certFiles) {
certStreams.add(new FileInputStream(certFile));
}
Expand Down
15 changes: 10 additions & 5 deletions sdk/src/main/java/io/opentdf/platform/sdk/TDF.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.*;
Expand Down Expand Up @@ -188,7 +189,7 @@ PolicyObject createPolicyObject(List<Autoconfigure.AttributeValueFQN> 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) {
Expand All @@ -208,7 +209,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<byte[]> symKeys = new ArrayList<>();
Map<String, Config.KASInfo> latestKASInfo = new HashMap<>();
if (tdfConfig.splitPlan == null || tdfConfig.splitPlan.isEmpty()) {
// Default split plan: Split keys across all KASes
Expand Down Expand Up @@ -261,6 +261,7 @@ private void prepareManifest(Config.TDFConfig tdfConfig, SDK.KAS kas) {
}
}

List<byte[]> symKeys = new ArrayList<>(splitIDs.size());
for (String splitID : splitIDs) {
// Symmetric key
byte[] symKey = new byte[GCM_KEY_SIZE];
Expand Down Expand Up @@ -358,6 +359,10 @@ 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);
} // 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);

Expand Down Expand Up @@ -520,8 +525,7 @@ public TDFObject createTDF(InputStream payload,
tdfObject.manifest.payload.url = TDFWriter.TDF_PAYLOAD_FILE_NAME;
tdfObject.manifest.payload.isEncrypted = true;

List<Manifest.Assertion> signedAssertions = new ArrayList<>();

List<Manifest.Assertion> signedAssertions = new ArrayList<>(tdfConfig.assertionConfigList.size());
for (var assertionConfig : tdfConfig.assertionConfigList) {
var assertion = new Manifest.Assertion();
assertion.id = assertionConfig.id;
Expand Down Expand Up @@ -585,7 +589,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;

Expand Down
Loading

0 comments on commit cf6f932

Please sign in to comment.