Merge pull request #675 from DwayneJengSage/dev4

Workaround for DIAN-749: Android inv-arc app has wrong public key, take 2

src/main/java/org/sagebionetworks/bridge/services/UploadArchiveService.java

@@ -3,7 +3,12 @@
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 
+import java.io.BufferedInputStream;
+import java.io.BufferedOutputStream;
 import java.io.ByteArrayInputStream;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -130,16 +135,42 @@ public byte[] decrypt(String appId, byte[] bytes) throws BridgeServiceException
         try (ByteArrayInputStream byteArrayInputStream = new ByteArrayInputStream(bytes);
              InputStream decryptedStream = decrypt(appId, byteArrayInputStream)) {
             return ByteStreams.toByteArray(decryptedStream);
-        } catch (IOException ex) {
+        } catch (CertificateEncodingException | CMSException | IOException | WrongEncryptionKeyException ex) {
             throw new BridgeServiceException(ex);
         }
     }
 
+    /** Decrypts the specified file "from" to the file specified in "to". */
+    public void decrypt(String appId, File from, File to) {
+        // Validate inputs.
+        checkNotNull(appId);
+        checkArgument(StringUtils.isNotBlank(appId));
+        checkNotNull(from);
+        checkNotNull(to);
+
+        // Decrypt.
+        try (InputStream fromStream = new BufferedInputStream(new FileInputStream(from));
+                InputStream decryptedStream = decrypt(appId, fromStream);
+                OutputStream toStream = new BufferedOutputStream(new FileOutputStream(to))) {
+            ByteStreams.copy(decryptedStream, toStream);
+        } catch (CertificateEncodingException | CMSException | IOException | WrongEncryptionKeyException ex) {
+            // This is a workaround for DIAN-749, Android inv-arc app had wrong public key for encryption -nbrown 10/25/23
+            if (ex instanceof WrongEncryptionKeyException && appId.equals("inv-arc")) {
+                decrypt("arc", from, to);
+            } else {
+                throw new BridgeServiceException(ex);
+            }
+        }
+    }
+
     /**
      * Decrypts the specified data stream, using the encryption materials for the specified app, and returns the a
      * stream of decrypted data. The caller is responsible for closing both streams.
+     *
+     * Package-scoped because this should only be called directly by unit tests.
      */
-    public InputStream decrypt(String appId, InputStream source) {
+    InputStream decrypt(String appId, InputStream source) throws CertificateEncodingException, CMSException,
+            IOException, WrongEncryptionKeyException {
         // validate
         checkNotNull(appId);
         checkArgument(StringUtils.isNotBlank(appId));
@@ -149,16 +180,7 @@ public InputStream decrypt(String appId, InputStream source) {
         CmsEncryptor encryptor = getEncryptorForApp(appId);
 
         // decrypt
-        try {
-            return encryptor.decrypt(source);
-        } catch (CertificateEncodingException | CMSException | IOException | WrongEncryptionKeyException ex) {
-            // This is a workaround for DIAN-749, Android inv-arc app had wrong public key for encryption -nbrown 10/25/23
-            if (ex instanceof WrongEncryptionKeyException && appId.equals("inv-arc")) {
-                return decrypt("arc", source);
-            } else {
-                throw new BridgeServiceException(ex);
-            }
-        }
+        return encryptor.decrypt(source);
     }
 
     /**

src/main/java/org/sagebionetworks/bridge/upload/DecryptHandler.java

@@ -1,14 +1,8 @@
 package org.sagebionetworks.bridge.upload;
 
-import java.io.BufferedInputStream;
-import java.io.BufferedOutputStream;
 import java.io.File;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.OutputStream;
 import javax.annotation.Nonnull;
 
-import com.google.common.io.ByteStreams;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 
@@ -51,27 +45,9 @@ public void handle(@Nonnull UploadValidationContext context) throws UploadValida
         File outputFile = fileHelper.newFile(context.getTempDir(), outputFilename);
 
         // Decrypt - Stream from input file to output file.
-        // Note: Neither FileHelper nor CmsEncryptor introduce any buffering. Since we're creating and closing streams,
-        // it's our responsibility to add the buffered stream.
-        try (InputStream inputFileStream = getBufferedInputStream(fileHelper.getInputStream(context.getDataFile()));
-             InputStream decryptedInputFileStream = uploadArchiveService.decrypt(context.getAppId(),
-                     inputFileStream);
-             OutputStream outputFileStream = new BufferedOutputStream(fileHelper.getOutputStream(outputFile))) {
-            ByteStreams.copy(decryptedInputFileStream, outputFileStream);
-        } catch (IOException ex) {
-            throw new UploadValidationException("Error decrypting file: " + ex.getMessage(), ex);
-        }
+        uploadArchiveService.decrypt(context.getAppId(), context.getDataFile(), outputFile);
 
         // Set file in context.
         context.setDecryptedDataFile(outputFile);
     }
-
-    // This helper method wraps a stream inside a buffered stream. It exists because our unit tests use
-    // InMemoryFileHelper, which uses a ByteArrayInputStream, which ignores closing. But in Prod, we need to wrap it in
-    // a BufferedInputStream because the files can get big, and a closed BufferedInputStream breaks unit tests.
-    //
-    // Note that OutputStream has no such limitation, since InMemoryFileHelper intercepts the output.
-    InputStream getBufferedInputStream(InputStream inputStream) {
-        return new BufferedInputStream(inputStream);
-    }
 }

src/test/java/org/sagebionetworks/bridge/services/UploadArchiveServiceInvArcTest.java

@@ -0,0 +1,78 @@
+package org.sagebionetworks.bridge.services;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.testng.Assert.assertEquals;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.security.PrivateKey;
+import java.security.cert.CertificateEncodingException;
+import java.security.cert.X509Certificate;
+
+import com.google.common.cache.LoadingCache;
+import org.apache.commons.io.FileUtils;
+import org.springframework.core.io.ClassPathResource;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import org.sagebionetworks.bridge.crypto.BcCmsEncryptor;
+import org.sagebionetworks.bridge.crypto.CmsEncryptor;
+import org.sagebionetworks.bridge.crypto.PemUtils;
+
+// Unit test for DIAN-749, Android inv-arc app had wrong public key for encryption.
+@SuppressWarnings("unchecked")
+public class UploadArchiveServiceInvArcTest {
+    private static final String APP_ID_ARC = "arc";
+    private static final String APP_ID_INV_ARC = "inv-arc";
+    private static final byte[] PLAIN_TEXT_DATA = "This is my raw data".getBytes();
+
+    private static UploadArchiveService archiveService;
+
+    @BeforeClass
+    public static void before() throws Exception {
+        // Load encryptors. rsacert.pem and rsaprivkey.pem represent the arc app. rsacert2.pem and rsaprivkey2.pem
+        // represent the inv-arc app.
+        CmsEncryptor arcEncryptor = loadEncryptor("/cms/rsacert.pem", "/cms/rsaprivkey.pem");
+        CmsEncryptor invArcEncryptor = loadEncryptor("/cms/rsacert2.pem", "/cms/rsaprivkey2.pem");
+
+        // Mock encryptor cache.
+        LoadingCache<String, CmsEncryptor> mockEncryptorCache = mock(LoadingCache.class);
+        when(mockEncryptorCache.get(APP_ID_ARC)).thenReturn(arcEncryptor);
+        when(mockEncryptorCache.get(APP_ID_INV_ARC)).thenReturn(invArcEncryptor);
+
+        // Create archive service.
+        archiveService = new UploadArchiveService();
+        archiveService.setCmsEncryptorCache(mockEncryptorCache);
+        archiveService.setMaxNumZipEntries(1000000);
+        archiveService.setMaxZipEntrySize(1000000);
+    }
+
+    private static CmsEncryptor loadEncryptor(String publicKeyPath, String privateKeyPath)
+            throws CertificateEncodingException, IOException {
+        File certFile = new ClassPathResource(publicKeyPath).getFile();
+        byte[] certBytes = Files.readAllBytes(certFile.toPath());
+        X509Certificate cert = PemUtils.loadCertificateFromPem(new String(certBytes));
+
+        File privateKeyFile = new ClassPathResource(privateKeyPath).getFile();
+        byte[] privateKeyBytes = Files.readAllBytes(privateKeyFile.toPath());
+        PrivateKey privateKey = PemUtils.loadPrivateKeyFromPem(new String(privateKeyBytes));
+
+        return new BcCmsEncryptor(cert, privateKey);
+    }
+
+    @Test
+    public void testInvArc() throws Exception {
+        // Encrypt some data with the arc app and write it to a file.
+        byte[] arcEncryptedData = archiveService.encrypt(APP_ID_ARC, PLAIN_TEXT_DATA);
+        File arcEncryptedFile = File.createTempFile("arc", ".encrypted");
+        FileUtils.writeByteArrayToFile(arcEncryptedFile, arcEncryptedData);
+
+        // Attempt to decrypt it with the inv-arc app.
+        File arcDecryptedFile = File.createTempFile("arc", ".decrypted");
+        archiveService.decrypt(APP_ID_INV_ARC, arcEncryptedFile, arcDecryptedFile);
+        byte[] result = FileUtils.readFileToByteArray(arcDecryptedFile);
+        assertEquals(result, PLAIN_TEXT_DATA);
+    }
+}

src/test/java/org/sagebionetworks/bridge/services/UploadArchiveServiceTest.java

@@ -140,7 +140,7 @@ public void decryptStreamBlankAppId() throws Exception {
     }
 
     @Test(expectedExceptions = NullPointerException.class)
-    public void decryptStreamNullBytes() {
+    public void decryptStreamNullBytes() throws Exception {
         archiveService.decrypt(TEST_APP_ID, (InputStream) null);
     }
 

src/test/java/org/sagebionetworks/bridge/upload/DecryptHandlerTest.java

@@ -1,23 +1,19 @@
 package org.sagebionetworks.bridge.upload;
 
+import static org.mockito.ArgumentMatchers.same;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.verifyZeroInteractions;
 import static org.testng.Assert.assertEquals;
 import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.eq;
 import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
 import static org.sagebionetworks.bridge.TestConstants.TEST_APP_ID;
 import static org.testng.Assert.assertSame;
 
-import java.io.ByteArrayInputStream;
 import java.io.File;
-import java.io.InputStream;
 
 import com.google.common.base.Charsets;
 
-import com.google.common.io.ByteStreams;
-import org.mockito.ArgumentCaptor;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
@@ -31,6 +27,8 @@
 
 public class DecryptHandlerTest {
     private static final byte[] DATA_FILE_CONTENT = "encrypted test data".getBytes(Charsets.UTF_8);
+    private static final String DECRYPTED_DATA_FILE_STRING = "decrypted test data";
+    private static final byte[] DECRYPTED_DATA_FILE_BYTES = DECRYPTED_DATA_FILE_STRING.getBytes(Charsets.UTF_8);
 
     private UploadValidationContext ctx;
     private File dataFile;
@@ -66,11 +64,11 @@ public void before() {
         ctx.setDataFile(dataFile);
 
         // mock UploadArchiveService
-        when(mockSvc.decrypt(eq(TEST_APP_ID), any(InputStream.class))).thenReturn(new ByteArrayInputStream(
-                "decrypted test data".getBytes(Charsets.UTF_8)));
-
-        // Don't actually buffer the input stream, as this breaks the test.
-        doAnswer(invocation -> invocation.getArgument(0)).when(handler).getBufferedInputStream(any());
+        doAnswer(invocation -> {
+            File decryptedFile = invocation.getArgument(2);
+            fileHelper.writeBytes(decryptedFile, DECRYPTED_DATA_FILE_BYTES);
+            return null;
+        }).when(mockSvc).decrypt(eq(TEST_APP_ID), same(dataFile), any(File.class));
     }
 
     @Test
@@ -81,13 +79,10 @@ public void test() throws Exception {
         // execute and validate
         handler.handle(ctx);
         byte[] decryptedContent = fileHelper.getBytes(ctx.getDecryptedDataFile());
-        assertEquals(new String(decryptedContent, Charsets.UTF_8), "decrypted test data");
+        assertEquals(new String(decryptedContent, Charsets.UTF_8), DECRYPTED_DATA_FILE_STRING);
 
         // Verify the correct file data was passed into the decryptor.
-        ArgumentCaptor<InputStream> encryptedInputStreamCaptor = ArgumentCaptor.forClass(InputStream.class);
-        verify(mockSvc).decrypt(eq(TEST_APP_ID), encryptedInputStreamCaptor.capture());
-        InputStream encryptedInputStream = encryptedInputStreamCaptor.getValue();
-        assertEquals(ByteStreams.toByteArray(encryptedInputStream), DATA_FILE_CONTENT);
+        verify(mockSvc).decrypt(eq(TEST_APP_ID), same(dataFile), any(File.class));
     }
 
     @Test

src/test/java/org/sagebionetworks/bridge/upload/UploadHandlersEndToEndTest.java

@@ -12,7 +12,6 @@
 import static org.testng.Assert.assertTrue;
 
 import java.io.File;
-import java.io.InputStream;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -231,8 +230,14 @@ private void test(UploadSchema schema, Survey survey, Map<String, String> fileMa
 
         // set up DecryptHandler - For ease of tests, this will just return the input verbatim.
         UploadArchiveService mockUploadArchiveService = mock(UploadArchiveService.class);
-        when(mockUploadArchiveService.decrypt(eq(TEST_APP_ID), any(InputStream.class)))
-                .thenAnswer(invocation -> invocation.getArgument(1));
+        doAnswer(invocation -> {
+            File encryptedFile = invocation.getArgument(1);
+            byte[] content = inMemoryFileHelper.getBytes(encryptedFile);
+
+            File decryptedFile = invocation.getArgument(2);
+            inMemoryFileHelper.writeBytes(decryptedFile, content);
+            return null;
+        }).when(mockUploadArchiveService).decrypt(eq(TEST_APP_ID), any(File.class), any(File.class));
 
         DecryptHandler decryptHandler = new DecryptHandler();
         decryptHandler.setFileHelper(inMemoryFileHelper);

src/test/resources/cms/rsacert2.pem

@@ -0,0 +1,28 @@
+-----BEGIN CERTIFICATE-----
+MIIExDCCA6ygAwIBAgIGC0ijqD17MA0GCSqGSIb3DQEBBQUAMIGeMQswCQYDVQQG
+EwJVUzELMAkGA1UECAwCV0ExEDAOBgNVBAcMB1NlYXR0bGUxGTAXBgNVBAoMEFNh
+Z2UgQmlvbmV0d29ya3MxDzANBgNVBAsMBkJyaWRnZTEkMCIGCSqGSIb3DQEJARYV
+YnJpZGdlSVRAc2FnZWJhc2Uub3JnMR4wHAYDVQQDDBVodHRwOi8vbG9jYWxob3N0
+OjkwMDAwHhcNMjExMjE0MjIzMDE1WhcNNDkwNTAyMjIzMDE1WjCBnjELMAkGA1UE
+BhMCVVMxCzAJBgNVBAgMAldBMRAwDgYDVQQHDAdTZWF0dGxlMRkwFwYDVQQKDBBT
+YWdlIEJpb25ldHdvcmtzMQ8wDQYDVQQLDAZCcmlkZ2UxJDAiBgkqhkiG9w0BCQEW
+FWJyaWRnZUlUQHNhZ2ViYXNlLm9yZzEeMBwGA1UEAwwVaHR0cDovL2xvY2FsaG9z
+dDo5MDAwMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAthO/+Nx+cdiQ
+a58/YcNmljKCcMbtvvKdSKbsBR8yHcMoWgINDQISnW8HYRJfFyIe97mpQqcdN7Xe
+BIofxjqYATON8RXswKHp/4EuqZ716YrWwZHSbuOWlLGEVUSf8jQ0co7y13STGYIM
+Aw7tZ8+Xf0zo8Kaf9xEUKX86U8PisyYigO53v7iLylphG5rzLPmdQEujTXgiAxLB
+e7Uk8RSxOiTuJTNGfPu3DFvut0ya187XRI5YAFkORZkATBi2JIObrvKzTp0CIOMq
+EPdQ82HY3SLwTW5XirzMgb1YuBKScW0Ag6QeRxEIwvKzinffC6EoaIl5fbTxGFh7
+EoLLMH63sQIDAQABo4IBBDCCAQAwDAYDVR0TBAUwAwEB/zCB0AYDVR0jBIHIMIHF
+gBQ2+u984fAH2xrWtkQjcYzT4TDki6GBpKSBoTCBnjELMAkGA1UEBhMCVVMxCzAJ
+BgNVBAgMAldBMRAwDgYDVQQHDAdTZWF0dGxlMRkwFwYDVQQKDBBTYWdlIEJpb25l
+dHdvcmtzMQ8wDQYDVQQLDAZCcmlkZ2UxJDAiBgkqhkiG9w0BCQEWFWJyaWRnZUlU
+QHNhZ2ViYXNlLm9yZzEeMBwGA1UEAwwVaHR0cDovL2xvY2FsaG9zdDo5MDAwggYL
+SKOoPXswHQYDVR0OBBYEFDb673zh8AfbGta2RCNxjNPhMOSLMA0GCSqGSIb3DQEB
+BQUAA4IBAQBTIHe2PH6/7DmtEMBEHoNcmT21JAfiyJPIYLLsloRDt9jAUduYWiVp
+eUU4y2nslY7nMML1wIm4D/zTlY/FkyvTC3CG/fyRb7ojqTOTPHqxaoG8apVUTKH9
+BoMzQ41LqHbz44Ht/QMqQVwUcEddmAGKFIifVBgCzL/NvwAKgkrQOFj5R751lScE
+x9jir8xFUNAYw3Ax8NlEI6ay22u5wv89EoShU2EUbbW/CdpqMvLpo6j66D5fihl8
+dKxv7JLTyYYMDDC7T9OU+1kF9fXKJYcrhciMTq5fHFnbiYFTtg/h9qJx35/27SZ6
+cplcxo4YdWY3kSargw4L2Wvo6L5nAORN
+-----END CERTIFICATE-----

src/test/resources/cms/rsaprivkey2.pem

@@ -0,0 +1,28 @@
+-----BEGIN RSA PRIVATE KEY-----
+MIIEvwIBADANBgkqhkiG9w0BAQEFAASCBKkwggSlAgEAAoIBAQC2E7/43H5x2JBr
+nz9hw2aWMoJwxu2+8p1IpuwFHzIdwyhaAg0NAhKdbwdhEl8XIh73ualCpx03td4E
+ih/GOpgBM43xFezAoen/gS6pnvXpitbBkdJu45aUsYRVRJ/yNDRyjvLXdJMZggwD
+Du1nz5d/TOjwpp/3ERQpfzpTw+KzJiKA7ne/uIvKWmEbmvMs+Z1AS6NNeCIDEsF7
+tSTxFLE6JO4lM0Z8+7cMW+63TJrXztdEjlgAWQ5FmQBMGLYkg5uu8rNOnQIg4yoQ
+91DzYdjdIvBNbleKvMyBvVi4EpJxbQCDpB5HEQjC8rOKd98LoShoiXl9tPEYWHsS
+gsswfrexAgMBAAECggEBAIyHVcWnuNf5hA3sjSjRfZ4zQcX1Y43bB1YJr2SMnUun
+Ur+VkakWjnOAPDvJyCa8qRYd7+uHu99BuSfby4ZdtvBGcClA+Mf8r/QKKo+0Jqyo
+AfTIrZf0hEYjdLWzD5gKfuhkOD3etaIcY1UA8m8LJCyWmbsTf6dbQSp+DfCU4aXO
+XTVi9ytPYrCPPI30wubDXIgUrfRkWVEpHQharJmR3P/4c2O2MapaXVz/64VQ0C4v
+d/wODYGQFNn9asRLd6H+UNflyyonpEL1X4t8ZZxw9CQrDH2zjc3eQSVY5d8N6wD5
+v9ca1irRHgW5bJsuKgrECBzs/qSNdeHgnciAi4YvOqECgYEA5WaNqJuLMIFnTId3
+VK55mF4c5zgKtjOoB8sJjB5izzZaRQYpbqu3ttQwPOlQAWrvcKGf37RQUMfpqgTB
+nOnCHoCrMWcWYWq8Ql8rpne0CDggad0wwMfnVK8SyG0RR0/tRE1hhn0llMKVHrJd
+tI9l3f/kFl/2bmFmrygqiRZQX6UCgYEAyzB2wE+ZN/BHlbYc1TonX2OEgoYI/YgV
+b1XV2iuqnL1Ntzh6Q1pKFfHgM2HrsA2s4/D5K9ic79kyuqwBDN+BME0//czUYl2y
+NDQmNqFr9QnKVWceyiToR7GoWdDPqgWRbLVCeaLz9i2yCgkhUO9KPy2twqlmO56r
+RFVTC49uuh0CgYEAx5v+85G1AdX3zq2pdjQDdkOeHsuy9mvocC+J9TSTgf6neZws
+/THKP/pOpxHVHgawpm7csEk0AbaSafCNkD4PPX90dx5eaRH5Ej/Bua47J1O/UJ65
+R2YqspNMYr4U5Np1eJNkoyPOSa0vGHDX/L8yQoPhMl76DX4PXaYzrOmPskkCgYB3
+42+eBxljrS2/w0V99qM2oFSWYxm45munVqEo6qzvcK8DVZqmVQbzrdTY3IUhSuBh
+WlTbLyNiTeilxkmUW+gxJNOGIC6Mn7Y/ISoO/+3gFlfBTmgXY/F+I/AulouBSWnG
+F6lSdfi2n722OC7lP1uyrXQiMKu2r+dkGWg3oPj3bQKBgQCzZy0j/D9ngwsulrci
+LjpNUicFz4AxzdP7lYyHlWN3fK+7YQYxKW3RROwnZKm+CfasQuwjeTsGuhYBjWaw
+DQggYbXHowjYoqfC3s+lziyqnysssyewsTeqIB3UzfaXOm55lS2yiMiDmg1MQ98G
+xwqPPjpkjTuCdxKZbd25hme7/Q==
+-----END RSA PRIVATE KEY-----