Add fix for NPE on encrypt/decrypt (#170)

* Add fix for NPE on encrypt/decrypt

* Remove useless test

* Bump versions

Author: coltfred

examples/cached-key-example/pom.xml

@@ -30,7 +30,7 @@
     <dependency>
       <groupId>com.ironcorelabs</groupId>
       <artifactId>tenant-security-java</artifactId>
-      <version>8.1.0</version>
+      <version>8.1.1</version>
     </dependency>
   </dependencies>
 
@@ -82,4 +82,4 @@
       </plugin>
     </plugins>
   </build>
-</project>
+</project>

pom.xml

@@ -8,7 +8,7 @@
   <groupId>com.ironcorelabs</groupId>
   <artifactId>tenant-security-java</artifactId>
   <packaging>jar</packaging>
-  <version>8.1.0</version>
+  <version>8.1.1</version>
   <name>tenant-security-java</name>
   <url>https://ironcorelabs.com/docs</url>
   <description>Java client library for the IronCore Labs Tenant Security Proxy.</description>

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

@@ -77,8 +77,12 @@ public static CompletableFuture<Void> encryptStreamInternal(byte[] documentKey,
         output.write(headerBytes);
         output.write(iv);
         while ((bytesRead = readNBytes(input, STREAM_CHUNKING)).length != 0) {
+          // Cipher.update may return null per its Javadoc. BC-FIPS buffers AEAD data until
+          // doFinal so ciphertext and tag are released together.
           byte[] encryptedBytes = cipher.update(bytesRead);
-          output.write(encryptedBytes);
+          if (encryptedBytes != null) {
+            output.write(encryptedBytes);
+          }
         }
         // Final bytes, which might be buffered data or just the GCM tag.
         byte[] finalBytes = cipher.doFinal();
@@ -104,7 +108,10 @@ public static CompletableFuture<Void> decryptStreamInternal(byte[] documentKey,
           Cipher cipher = getNewAesCipher(documentKey, iv, false);
           byte[] currentChunk = new byte[0];
           while ((currentChunk = readNBytes(encryptedStream, STREAM_CHUNKING)).length > 0) {
-            decryptedStream.write(cipher.update(currentChunk));
+            byte[] decryptedChunk = cipher.update(currentChunk);
+            if (decryptedChunk != null) {
+              decryptedStream.write(decryptedChunk);
+            }
           }
           decryptedStream.write(cipher.doFinal());
           return null;

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

@@ -61,7 +61,7 @@ private static String stripTrailingSlash(String s) {
   private final int connectTimeout;
 
   // TSC version that will be sent to the TSP.
-  static final String sdkVersion = "8.1.0";
+  static final String sdkVersion = "8.1.1";
 
   TenantSecurityRequest(String tspDomain, String apiKey, int requestThreadSize, int timeout) {
     this(tspDomain, apiKey, requestThreadSize, timeout, timeout);

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

@@ -0,0 +1,121 @@
+package com.ironcorelabs.tenantsecurity.kms.v1;
+
+import java.io.ByteArrayOutputStream;
+import java.security.AlgorithmParameters;
+import java.security.InvalidAlgorithmParameterException;
+import java.security.InvalidKeyException;
+import java.security.Key;
+import java.security.Provider;
+import java.security.SecureRandom;
+import java.security.spec.AlgorithmParameterSpec;
+import javax.crypto.BadPaddingException;
+import javax.crypto.Cipher;
+import javax.crypto.CipherSpi;
+import javax.crypto.IllegalBlockSizeException;
+import javax.crypto.ShortBufferException;
+
+// Test-only JCE provider used to reproduce the BC-FIPS AEAD buffering behavior
+// that surfaces in issue #167. Every engineUpdate call buffers input and returns
+// null; buffered bytes are released only at engineDoFinal. See
+// CryptoUtilsTest#streamingRoundtripWithBufferingProvider.
+public class BufferingGcmProvider extends Provider {
+  public BufferingGcmProvider() {
+    super("BufferingGcmTest", "1.0", "Test provider buffering GCM data like BC-FIPS");
+    put("Cipher.AES/GCM/NoPadding", BufferingGcmCipherSpi.class.getName());
+  }
+
+  public static class BufferingGcmCipherSpi extends CipherSpi {
+    private final Cipher delegate;
+    private final ByteArrayOutputStream buffer = new ByteArrayOutputStream();
+
+    public BufferingGcmCipherSpi() throws Exception {
+      delegate = Cipher.getInstance("AES/GCM/NoPadding", "SunJCE");
+    }
+
+    @Override
+    protected void engineSetMode(String mode) {}
+
+    @Override
+    protected void engineSetPadding(String padding) {}
+
+    @Override
+    protected int engineGetBlockSize() {
+      return delegate.getBlockSize();
+    }
+
+    @Override
+    protected int engineGetOutputSize(int inputLen) {
+      return delegate.getOutputSize(inputLen + buffer.size());
+    }
+
+    @Override
+    protected byte[] engineGetIV() {
+      return delegate.getIV();
+    }
+
+    @Override
+    protected AlgorithmParameters engineGetParameters() {
+      return delegate.getParameters();
+    }
+
+    @Override
+    protected void engineInit(int opmode, Key key, SecureRandom random) throws InvalidKeyException {
+      buffer.reset();
+      delegate.init(opmode, key, random);
+    }
+
+    @Override
+    protected void engineInit(int opmode, Key key, AlgorithmParameterSpec params,
+        SecureRandom random) throws InvalidKeyException, InvalidAlgorithmParameterException {
+      buffer.reset();
+      delegate.init(opmode, key, params, random);
+    }
+
+    @Override
+    protected void engineInit(int opmode, Key key, AlgorithmParameters params, SecureRandom random)
+        throws InvalidKeyException, InvalidAlgorithmParameterException {
+      buffer.reset();
+      delegate.init(opmode, key, params, random);
+    }
+
+    @Override
+    protected byte[] engineUpdate(byte[] input, int inputOffset, int inputLen) {
+      if (input != null && inputLen > 0) {
+        buffer.write(input, inputOffset, inputLen);
+      }
+      return null;
+    }
+
+    @Override
+    protected int engineUpdate(byte[] input, int inputOffset, int inputLen, byte[] output,
+        int outputOffset) {
+      if (input != null && inputLen > 0) {
+        buffer.write(input, inputOffset, inputLen);
+      }
+      return 0;
+    }
+
+    @Override
+    protected byte[] engineDoFinal(byte[] input, int inputOffset, int inputLen)
+        throws IllegalBlockSizeException, BadPaddingException {
+      if (input != null && inputLen > 0) {
+        buffer.write(input, inputOffset, inputLen);
+      }
+      byte[] all = buffer.toByteArray();
+      buffer.reset();
+      return delegate.doFinal(all);
+    }
+
+    @Override
+    protected int engineDoFinal(byte[] input, int inputOffset, int inputLen, byte[] output,
+        int outputOffset)
+        throws ShortBufferException, IllegalBlockSizeException, BadPaddingException {
+      byte[] result = engineDoFinal(input, inputOffset, inputLen);
+      if (output.length - outputOffset < result.length) {
+        throw new ShortBufferException();
+      }
+      System.arraycopy(result, 0, output, outputOffset, result.length);
+      return result.length;
+    }
+  }
+}

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

@@ -1,13 +1,15 @@
 package com.ironcorelabs.tenantsecurity.kms.v1;
 
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNull;
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.nio.ByteBuffer;
 import java.security.SecureRandom;
 import java.util.Arrays;
 import java.util.stream.IntStream;
+import javax.crypto.Cipher;
 import org.testng.annotations.Test;
 
 @Test(groups = {"unit"})
@@ -259,4 +261,29 @@ public void getNBytesRequestMoreOnEmpty() throws Exception {
     byte[] buffer = new byte[0];
     assertEquals(CryptoUtils.readNBytes(new ByteArrayInputStream(buffer), 10), new byte[0]);
   }
+
+  // Regression for issue #167. Simulates BC-FIPS GCM behavior with a custom JCE
+  // provider that buffers AEAD data and returns null from every Cipher.update call
+  // until doFinal. Before the fix, encryptStreamInternal NPE'd on output.write(null).
+  public void streamingRoundtripWithBufferingProvider() throws Exception {
+    java.security.Provider provider = new BufferingGcmProvider();
+    java.security.Security.insertProviderAt(provider, 1);
+    try {
+      byte[] documentKey = new byte[32];
+      secureRandom.nextBytes(documentKey);
+      byte[] plaintext = "foo".getBytes("UTF-8");
+      ByteArrayInputStream inputStream = new ByteArrayInputStream(plaintext);
+      ByteArrayOutputStream encryptOutputStream = new ByteArrayOutputStream();
+      CryptoUtils.encryptStreamInternal(documentKey, metadata, inputStream, encryptOutputStream,
+          secureRandom).get();
+      byte[] encryptedBytes = encryptOutputStream.toByteArray();
+      ByteArrayOutputStream decryptedStream = new ByteArrayOutputStream();
+      ByteArrayInputStream encryptedStream = new ByteArrayInputStream(encryptedBytes);
+      CryptoUtils.decryptStreamInternal(documentKey, encryptedStream, decryptedStream).get();
+      assertEquals(decryptedStream.toByteArray(), plaintext);
+    } finally {
+      java.security.Security.removeProvider(provider.getName());
+    }
+  }
+
 }