Fix potential null pointer exceptions

.github/workflows/ci.yml

@@ -47,7 +47,10 @@ jobs:
           gcloud-auth: ${{ secrets.GCLOUD_AUTH }}
           env-file-path: .env.integration
       - name: integration test
-        run: env $(cat .env.integration) ./test-suites/integrationTest.sh
+        run: env $(cat .env.integration) ./test-suites/allTest.sh
+        env:
+          TENANT_ID: INTEGRATION-TEST-GCP
+          NEW_TENANT_ID: INTEGRATION-TEST-AWS
 
   build_examples:
     runs-on: ubuntu-22.04

src/main/java/com/ironcorelabs/tenantsecurity/kms/v1/BatchDocumentKeys.java

@@ -1,14 +1,13 @@
 package com.ironcorelabs.tenantsecurity.kms.v1;
 
 import java.util.Map;
-
 import com.google.api.client.util.Key;
 
 /**
  * A map from a document ID to a either the wrapped or unwrapped version of a documents keys. Also
  * includes a map of failures if any problems occurred when performing the batch wrap operation.
  */
-public class BatchDocumentKeys<T> {
+public class BatchDocumentKeys<T> extends NullParsingValidator {
   @Key
   private Map<String, T> keys;
 
@@ -22,4 +21,11 @@ public Map<String, T> getKeys() {
   public Map<String, ErrorResponse> getFailures() {
     return this.failures;
   }
+
+  @Override
+  void ensureNoNullsOrThrow() throws IllegalArgumentException {
+    if (keys == null || failures == null)
+      throw new IllegalArgumentException(
+          "Batch response from the Tenant Security Proxy was not valid.");
+  }
 }

src/main/java/com/ironcorelabs/tenantsecurity/kms/v1/DeriveKeyResponse.java

@@ -5,7 +5,7 @@
 import com.google.api.client.util.Key;
 import com.ironcorelabs.tenantsecurity.kms.v1.exception.TspServiceException;
 
-public final class DeriveKeyResponse {
+public final class DeriveKeyResponse extends NullParsingValidator {
   @Key
   private boolean hasPrimaryConfig;
   @Key
@@ -37,4 +37,10 @@ CompletableFuture<DerivedKey[]> getDerivedKeys(String secretPath, String derivat
     }
     return CompletableFuture.completedFuture(derivedKeys);
   }
+
+  @Override
+  void ensureNoNullsOrThrow() throws IllegalArgumentException {
+    if (derivedKeys == null)
+      throw new IllegalArgumentException("TSP failed to derive keys.");
+  }
 }

src/main/java/com/ironcorelabs/tenantsecurity/kms/v1/DeterministicCryptoUtils.java

@@ -106,7 +106,7 @@ static CompletableFuture<DeterministicPlaintextField> decryptField(
           return key;
         }).thenCompose(key -> decryptBytes(parts.getEncryptedBytes(), key.getDerivedKeyBytes())))
         .thenApply(decrypted -> new DeterministicPlaintextField(decrypted,
-            encryptedField.getDerivationPath(), encryptedField.getSecretPath()));
+            encryptedField.getSecretPath(), encryptedField.getDerivationPath()));
   }
 
 

src/main/java/com/ironcorelabs/tenantsecurity/kms/v1/NullParsingValidator.java

@@ -0,0 +1,8 @@
+package com.ironcorelabs.tenantsecurity.kms.v1;
+
+abstract class NullParsingValidator {
+  /**
+   * Throws an IllegalArgumentException if any of the fields were parsed as null.
+   */
+  abstract void ensureNoNullsOrThrow() throws IllegalArgumentException;
+}

src/main/java/com/ironcorelabs/tenantsecurity/kms/v1/RekeyedDocumentKey.java

@@ -5,11 +5,18 @@
 /**
  * An EDEK made by wrapping an existing encrypted document with a tenant's KMS, in Base64 format.
  */
-public class RekeyedDocumentKey {
+public class RekeyedDocumentKey extends NullParsingValidator {
   @Key
   private String edek;
 
   public String getEdek() {
     return this.edek;
   }
+
+  @Override
+  void ensureNoNullsOrThrow() throws IllegalArgumentException {
+    if (edek == null)
+      throw new IllegalArgumentException(
+          "Rekeyed document key response from the Tenant Security Proxy was not valid base64.");
+  }
 }

src/main/java/com/ironcorelabs/tenantsecurity/kms/v1/TenantSecurityClient.java

@@ -189,7 +189,7 @@ public Builder allowInsecureHttp(boolean allow) {
     }
 
     /**
-     * Construct the TenantSecurityClient fron the builder.
+     * Construct the TenantSecurityClient from the builder.
      *
      * @return The newly constructed TenantSecurityClient.
      * @throws Exception If the tsp url isn't valid or if HTTPS is required and not provided.

src/main/java/com/ironcorelabs/tenantsecurity/kms/v1/TenantSecurityRequest.java

@@ -153,13 +153,42 @@ enum SecretType {
    * Generic method for making a request to the provided URL with the provided post data. Returns an
    * instance of the provided generic JSON class or an error message with the provided error.
    */
-  private <T> CompletableFuture<T> makeRequestAndParseFailure(GenericUrl url,
-      Map<String, Object> postData, Class<T> jsonType, String errorMessage) {
+  private <T extends NullParsingValidator> CompletableFuture<T> makeRequestAndParseFailure(
+      GenericUrl url, Map<String, Object> postData, Class<T> jsonType, String errorMessage) {
     return CompletableFuture.supplyAsync(() -> {
       try {
         HttpResponse resp = this.getApiRequest(postData, url).execute();
         if (resp.isSuccessStatusCode()) {
-          return resp.parseAs(jsonType);
+          T parsed = resp.parseAs(jsonType);
+          parsed.ensureNoNullsOrThrow();
+          return parsed;
+        }
+        throw parseFailureFromRequest(resp);
+      } catch (Exception cause) {
+        if (cause instanceof TenantSecurityException) {
+          throw new CompletionException(cause);
+        } else if (cause instanceof IllegalArgumentException) {
+          throw new CompletionException(new TspServiceException(
+              TenantSecurityErrorCodes.UNKNOWN_ERROR, 0, errorMessage, cause));
+        }
+        throw new CompletionException(new TspServiceException(
+            TenantSecurityErrorCodes.UNABLE_TO_MAKE_REQUEST, 0, errorMessage, cause));
+      }
+    }, webRequestExecutor);
+  }
+
+  /**
+   * Overload for generic method for making a request to the provided URL with the provided post
+   * data. Returns a CompletableFuture<Void> because it does not try to parse a successful result.
+   * In the case of an error, it does try to parse the provided error.
+   */
+  private CompletableFuture<Void> makeRequestAndParseFailure(GenericUrl url,
+      Map<String, Object> postData, String errorMessage) {
+    return CompletableFuture.supplyAsync(() -> {
+      try {
+        HttpResponse resp = this.getApiRequest(postData, url).execute();
+        if (resp.isSuccessStatusCode()) {
+          return null;
         }
         throw parseFailureFromRequest(resp);
       } catch (Exception cause) {
@@ -261,7 +290,7 @@ CompletableFuture<Void> logSecurityEvent(SecurityEvent event, EventMetadata meta
     String error = String.format(
         "Unable to make request to Tenant Security Proxy security event endpoint. Endpoint requested: %s",
         this.securityEventEndpoint);
-    return this.makeRequestAndParseFailure(this.securityEventEndpoint, postData, Void.class, error);
+    return this.makeRequestAndParseFailure(this.securityEventEndpoint, postData, error);
   }
 
   /**

src/main/java/com/ironcorelabs/tenantsecurity/kms/v1/UnwrappedDocumentKey.java

@@ -7,7 +7,7 @@
 /**
  * Represents the JSON response object from the document/unwrap endpoint which includes the dek.
  */
-public class UnwrappedDocumentKey {
+public class UnwrappedDocumentKey extends NullParsingValidator {
   @Key
   private String dek;
 
@@ -19,4 +19,11 @@ public byte[] getDekBytes() {
           "Unwrap DEK response from the Tenant Security Proxy was not valid base64.");
     }
   }
+
+  @Override
+  void ensureNoNullsOrThrow() throws IllegalArgumentException {
+    if (dek == null)
+      throw new IllegalArgumentException(
+          "Unwrap DEK response from the Tenant Security Proxy was not valid base64.");
+  }
 }

src/main/java/com/ironcorelabs/tenantsecurity/kms/v1/WrappedDocumentKey.java

@@ -7,7 +7,7 @@
 /**
  * A new DEK wrapped by the tenant's KMS and its encrypted form (EDEK), both in Base64 format.
  */
-public class WrappedDocumentKey {
+public class WrappedDocumentKey extends NullParsingValidator {
   @Key
   private String dek;
 
@@ -26,4 +26,11 @@ public byte[] getDekBytes() {
   public String getEdek() {
     return this.edek;
   }
+
+  @Override
+  void ensureNoNullsOrThrow() throws IllegalArgumentException {
+    if (edek == null || dek == null)
+      throw new IllegalArgumentException(
+          "Wrapped document key response from the Tenant Security Proxy was not valid base64.");
+  }
 }

src/test/java/com/ironcorelabs/tenantsecurity/kms/v1/JsonParsingTest.java

@@ -0,0 +1,52 @@
+package com.ironcorelabs.tenantsecurity.kms.v1;
+
+import java.io.StringReader;
+import org.testng.annotations.Test;
+import com.google.api.client.json.JsonObjectParser;
+import com.google.api.client.json.gson.GsonFactory;
+
+@Test(groups = {"unit"})
+public class JsonParsingTest {
+  static JsonObjectParser parser = new JsonObjectParser(new GsonFactory());
+
+  @Test(expectedExceptions = IllegalArgumentException.class)
+  void batchKeysErrors() throws Exception {
+    String json = "{}";
+    BatchWrappedDocumentKeys type = parser.<BatchWrappedDocumentKeys>parseAndClose(
+        new StringReader(json), BatchWrappedDocumentKeys.class);
+    type.ensureNoNullsOrThrow();
+  }
+
+  @Test(expectedExceptions = IllegalArgumentException.class)
+  void derivedKeyResponseErrors() throws Exception {
+    String json = "{}";
+    DeriveKeyResponse type =
+        parser.<DeriveKeyResponse>parseAndClose(new StringReader(json), DeriveKeyResponse.class);
+    type.ensureNoNullsOrThrow();
+
+  }
+
+  @Test(expectedExceptions = IllegalArgumentException.class)
+  void unwrappedDocumentKeyErrors() throws Exception {
+    String json = "{}";
+    UnwrappedDocumentKey type = parser.<UnwrappedDocumentKey>parseAndClose(new StringReader(json),
+        UnwrappedDocumentKey.class);
+    type.ensureNoNullsOrThrow();
+  }
+
+  @Test(expectedExceptions = IllegalArgumentException.class)
+  void wrappedDocumentKeyErrors() throws Exception {
+    String json = "{}";
+    WrappedDocumentKey type =
+        parser.<WrappedDocumentKey>parseAndClose(new StringReader(json), WrappedDocumentKey.class);
+    type.ensureNoNullsOrThrow();
+  }
+
+  @Test(expectedExceptions = IllegalArgumentException.class)
+  void rekeyedDocumentKeyErrors() throws Exception {
+    String json = "{}";
+    RekeyedDocumentKey type =
+        parser.<RekeyedDocumentKey>parseAndClose(new StringReader(json), RekeyedDocumentKey.class);
+    type.ensureNoNullsOrThrow();
+  }
+}

src/test/java/com/ironcorelabs/tenantsecurity/kms/v1/LocalBatch.java

@@ -60,7 +60,7 @@ public void batchRoundtrip() throws Exception {
 
     TenantSecurityClient client =
         new TenantSecurityClient.Builder(TestSettings.TSP_ADDRESS + TestSettings.TSP_PORT,
-            this.API_KEY).build();
+            this.API_KEY).allowInsecureHttp(true).build();
 
     int batchSize = 25;
     int batchRepetitions = 50;

src/test/java/com/ironcorelabs/tenantsecurity/kms/v1/LocalDeterministic.java

@@ -22,8 +22,8 @@ private void assertEqualBytes(byte[] one, byte[] two) throws Exception {
   }
 
   private DeterministicPlaintextField getRoundtripDataToEncrypt() {
-    return new DeterministicPlaintextField("Encrypt these bytes!".getBytes(), "deriv_path",
-        "secret_path");
+    return new DeterministicPlaintextField("Encrypt these bytes!".getBytes(), "secret_path",
+        "deriv_path");
   }
 
   private CompletableFuture<DeterministicTenantSecurityClient> createClient() {

src/test/java/com/ironcorelabs/tenantsecurity/kms/v1/LocalRoundTrip.java

@@ -12,7 +12,6 @@
 import com.ironcorelabs.tenantsecurity.kms.v1.exception.TenantSecurityException;
 import com.ironcorelabs.tenantsecurity.logdriver.v1.EventMetadata;
 import com.ironcorelabs.tenantsecurity.logdriver.v1.UserEvent;
-import com.ironcorelabs.tenantsecurity.utils.CompletableFutures;
 import org.testng.annotations.Test;
 
 @Test(groups = {"local-integration"})

src/test/java/com/ironcorelabs/tenantsecurity/kms/v1/NotPrimaryAndDisabledConfigs.java

@@ -9,7 +9,6 @@
 import java.util.concurrent.ExecutionException;
 import com.ironcorelabs.tenantsecurity.TestUtils;
 import com.ironcorelabs.tenantsecurity.kms.v1.exception.TenantSecurityException;
-import com.ironcorelabs.tenantsecurity.utils.CompletableFutures;
 import org.testng.annotations.Test;
 
 @Test(groups = {"dev-integration"})

src/test/java/com/ironcorelabs/tenantsecurity/kms/v1/RekeyRoundTrip.java

@@ -4,7 +4,6 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.CompletionException;
 import java.util.concurrent.ExecutionException;
 import com.ironcorelabs.tenantsecurity.kms.v1.exception.TenantSecurityException;
 import org.testng.annotations.Test;
@@ -50,24 +49,21 @@ public void roundtripTest() throws Exception {
         new DocumentMetadata(tenant_id, "integrationTest", "sample", customFields, "customRayID");
     Map<String, byte[]> documentMap = getRoundtripDataToEncrypt();
 
-    CompletableFuture<PlaintextDocument> roundtrip =
-        TenantSecurityClient.create(tsp_address + tsp_port, api_key).thenCompose(client -> {
-          try {
-            return client.encrypt(documentMap, metadata).thenCompose(encryptedDocument -> {
-              return client.rekeyEdek(encryptedDocument.getEdek(), metadata, new_tenant_id)
-                  .thenCompose(rekeyedEdek -> {
-                    DocumentMetadata newMetadata = new DocumentMetadata(new_tenant_id,
-                        "integrationTest", "sample", customFields, "customRayID");
-                    EncryptedDocument newDocument =
-                        new EncryptedDocument(encryptedDocument.getEncryptedFields(), rekeyedEdek);
-                    return client.decrypt(newDocument, newMetadata);
-                  });
-            });
-          } catch (Exception e) {
-            throw new CompletionException(e);
-          }
-        });
     try {
+      TenantSecurityClient client =
+          new TenantSecurityClient.Builder(tsp_address + tsp_port, api_key).allowInsecureHttp(true)
+              .build();
+      CompletableFuture<PlaintextDocument> roundtrip =
+          client.encrypt(documentMap, metadata).thenCompose(encryptedDocument -> {
+            return client.rekeyEdek(encryptedDocument.getEdek(), metadata, new_tenant_id)
+                .thenCompose(rekeyedEdek -> {
+                  DocumentMetadata newMetadata = new DocumentMetadata(new_tenant_id,
+                      "integrationTest", "sample", customFields, "customRayID");
+                  EncryptedDocument newDocument =
+                      new EncryptedDocument(encryptedDocument.getEncryptedFields(), rekeyedEdek);
+                  return client.decrypt(newDocument, newMetadata);
+                });
+          });
       Map<String, byte[]> decryptedValuesMap = roundtrip.get().getDecryptedFields();
       assertEqualBytes(decryptedValuesMap.get("doc1"), documentMap.get("doc1"));
       assertEqualBytes(decryptedValuesMap.get("doc2"), documentMap.get("doc2"));

test-suites/allTest.sh

@@ -0,0 +1,4 @@
+#!/bin/bash
+# Be sure to set API_KEY, TENANT_ID, and NEW_TENANT_ID env vars
+cd "${0%/*/*}" # set the current directory to the one above this script
+mvn -Dsuite=test-suites/test-all test

test-suites/test-all.xml

@@ -0,0 +1,18 @@
+<!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd">
+
+<suite name="allTests">
+  <test name="allTests">
+    <groups>
+      <run>
+        <include name="unit" />
+        <include name="local-integration" />
+        <include name="dev-integration" />
+        <include name="local-batch-integration" />
+        <include name="local-deterministic" />
+      </run>
+    </groups>
+    <packages>
+      <package name=".*" />
+    </packages>
+  </test>
+</suite>