Fix potential null pointer exceptions (#159)

Author: giarc3

.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.");
+  }
 }