Merge pull request #10713 from nextcloud/checkCSR

Check CSR

app/src/androidTest/java/com/nextcloud/client/EndToEndRandomIT.java

@@ -39,6 +39,8 @@ import com.owncloud.android.lib.resources.e2ee.ToggleEncryptionRemoteOperation;
 import com.owncloud.android.lib.resources.files.ReadFileRemoteOperation;
 import com.owncloud.android.lib.resources.status.OCCapability;
 import com.owncloud.android.lib.resources.status.OwnCloudVersion;
+import com.owncloud.android.lib.resources.users.DeletePrivateKeyOperation;
+import com.owncloud.android.lib.resources.users.DeletePublicKeyOperation;
 import com.owncloud.android.lib.resources.users.GetPrivateKeyOperation;
 import com.owncloud.android.lib.resources.users.GetPublicKeyOperation;
 import com.owncloud.android.lib.resources.users.SendCSROperation;
@@ -58,7 +60,10 @@ import org.junit.runner.RunWith;
 
 import java.io.File;
 import java.io.IOException;
+import java.math.BigInteger;
 import java.security.KeyPair;
+import java.security.interfaces.RSAPrivateCrtKey;
+import java.security.interfaces.RSAPublicKey;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Random;
@@ -474,6 +479,36 @@ public class EndToEndRandomIT extends AbstractOnServerIT {
         assertFalse(new File(uploadedFile.getStoragePath()).exists());
     }
 
+    @Test
+    public void testCheckCSR() throws Exception {
+        deleteKeys();
+
+        // Create public/private key pair
+        KeyPair keyPair = EncryptionUtils.generateKeyPair();
+
+        // create CSR
+        AccountManager accountManager = AccountManager.get(targetContext);
+        String userId = accountManager.getUserData(account, AccountUtils.Constants.KEY_USER_ID);
+        String urlEncoded = CsrHelper.generateCsrPemEncodedString(keyPair, userId);
+
+        SendCSROperation operation = new SendCSROperation(urlEncoded);
+        RemoteOperationResult result = operation.execute(account, targetContext);
+
+        assertTrue(result.isSuccess());
+        String publicKeyString = (String) result.getData().get(0);
+
+        // check key
+        RSAPrivateCrtKey privateKey = (RSAPrivateCrtKey) keyPair.getPrivate();
+        RSAPublicKey publicKey = EncryptionUtils.convertPublicKeyFromString(publicKeyString);
+
+        BigInteger modulusPublic = publicKey.getModulus();
+        BigInteger modulusPrivate = privateKey.getModulus();
+
+        assertEquals(modulusPrivate, modulusPublic);
+
+        createKeys();
+    }
+
     private void deleteFile(int i) {
         ArrayList<OCFile> files = new ArrayList<>();
         for (OCFile file : getStorageManager().getFolderContent(currentFolder, false)) {
@@ -529,11 +564,11 @@ public class EndToEndRandomIT extends AbstractOnServerIT {
     private void useExistingKeys() throws Exception {
         // download them from server
         GetPublicKeyOperation publicKeyOperation = new GetPublicKeyOperation();
-        RemoteOperationResult publicKeyResult = publicKeyOperation.execute(account, targetContext);
+        RemoteOperationResult<String> publicKeyResult = publicKeyOperation.execute(account, targetContext);
 
         assertTrue("Result code:" + publicKeyResult.getHttpCode(), publicKeyResult.isSuccess());
 
-        String publicKeyFromServer = (String) publicKeyResult.getData().get(0);
+        String publicKeyFromServer = publicKeyResult.getResultData();
         arbitraryDataProvider.storeOrUpdateKeyValue(account.name,
                                                     EncryptionUtils.PUBLIC_KEY,
                                                     publicKeyFromServer);
@@ -559,7 +594,9 @@ public class EndToEndRandomIT extends AbstractOnServerIT {
     TODO do not c&p code
      */
     private static void createKeys() throws Exception {
-        String publicKey;
+        deleteKeys();
+
+        String publicKeyString;
 
         // Create public/private key pair
         KeyPair keyPair = EncryptionUtils.generateKeyPair();
@@ -573,7 +610,18 @@ public class EndToEndRandomIT extends AbstractOnServerIT {
         RemoteOperationResult result = operation.execute(account, targetContext);
 
         if (result.isSuccess()) {
-            publicKey = (String) result.getData().get(0);
+            publicKeyString = (String) result.getData().get(0);
+
+            // check key
+            RSAPrivateCrtKey privateKey = (RSAPrivateCrtKey) keyPair.getPrivate();
+            RSAPublicKey publicKey = EncryptionUtils.convertPublicKeyFromString(publicKeyString);
+
+            BigInteger modulusPublic = publicKey.getModulus();
+            BigInteger modulusPrivate = privateKey.getModulus();
+
+            if (modulusPrivate.compareTo(modulusPublic) != 0) {
+                throw new RuntimeException("Wrong CSR returned");
+            }
         } else {
             throw new Exception("failed to send CSR", result.getException());
         }
@@ -591,7 +639,7 @@ public class EndToEndRandomIT extends AbstractOnServerIT {
         if (storePrivateKeyResult.isSuccess()) {
             arbitraryDataProvider.storeOrUpdateKeyValue(account.name, EncryptionUtils.PRIVATE_KEY,
                                                         privateKeyString);
-            arbitraryDataProvider.storeOrUpdateKeyValue(account.name, EncryptionUtils.PUBLIC_KEY, publicKey);
+            arbitraryDataProvider.storeOrUpdateKeyValue(account.name, EncryptionUtils.PUBLIC_KEY, publicKeyString);
             arbitraryDataProvider.storeOrUpdateKeyValue(account.name, EncryptionUtils.MNEMONIC,
                                                         generateMnemonicString());
         } else {
@@ -599,6 +647,21 @@ public class EndToEndRandomIT extends AbstractOnServerIT {
         }
     }
 
+    private static void deleteKeys() {
+        RemoteOperationResult<PrivateKey> privateKeyRemoteOperationResult = new GetPrivateKeyOperation().execute(client);
+        RemoteOperationResult<String> publicKeyRemoteOperationResult = new GetPublicKeyOperation().execute(client);
+
+        if (privateKeyRemoteOperationResult.isSuccess() || publicKeyRemoteOperationResult.isSuccess()) {
+            // delete keys
+            assertTrue(new DeletePrivateKeyOperation().execute(client).isSuccess());
+            assertTrue(new DeletePublicKeyOperation().execute(client).isSuccess());
+
+            arbitraryDataProvider.deleteKeyForAccount(account.name, EncryptionUtils.PRIVATE_KEY);
+            arbitraryDataProvider.deleteKeyForAccount(account.name, EncryptionUtils.PUBLIC_KEY);
+            arbitraryDataProvider.deleteKeyForAccount(account.name, EncryptionUtils.MNEMONIC);
+        }
+    }
+
     private static String generateMnemonicString() {
         return "1 2 3 4 5 6";
     }

app/src/androidTest/java/com/owncloud/android/util/EncryptionTestIT.java

@@ -45,11 +45,14 @@ import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.math.BigInteger;
 import java.security.KeyPair;
 import java.security.KeyPairGenerator;
 import java.security.MessageDigest;
 import java.security.PrivateKey;
 import java.security.SecureRandom;
+import java.security.interfaces.RSAPrivateCrtKey;
+import java.security.interfaces.RSAPublicKey;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -178,6 +181,18 @@ public class EncryptionTestIT {
         decryptStringAsymmetric(encryptedString, keyPair2.getPrivate());
     }
 
+    @Test
+    public void testModulus() throws Exception {
+        KeyPair keyPair = EncryptionUtils.generateKeyPair();
+        RSAPublicKey publicKey = (RSAPublicKey) keyPair.getPublic();
+        RSAPrivateCrtKey privateKey = (RSAPrivateCrtKey) keyPair.getPrivate();
+
+        BigInteger modulusPublic = publicKey.getModulus();
+        BigInteger modulusPrivate = privateKey.getModulus();
+
+        assertEquals(modulusPrivate, modulusPublic);
+    }
+
     @Test
     public void encryptStringSymmetricRandom() throws Exception {
         int max = 500;

app/src/main/java/com/owncloud/android/ui/dialog/SetupEncryptionDialogFragment.java

@@ -295,12 +295,12 @@ public class SetupEncryptionDialogFragment extends DialogFragment implements Inj
             //  - decrypt private key, store unencrypted private key in database
 
             GetPublicKeyOperation publicKeyOperation = new GetPublicKeyOperation();
-            RemoteOperationResult publicKeyResult = publicKeyOperation.execute(user, getContext());
+            RemoteOperationResult<String> publicKeyResult = publicKeyOperation.execute(user, getContext());
 
             if (publicKeyResult.isSuccess()) {
                 Log_OC.d(TAG, "public key successful downloaded for " + user.getAccountName());
 
-                String publicKeyFromServer = (String) publicKeyResult.getData().get(0);
+                String publicKeyFromServer = publicKeyResult.getResultData();
                 arbitraryDataProvider.storeOrUpdateKeyValue(user.getAccountName(),
                                                             EncryptionUtils.PUBLIC_KEY,
                                                             publicKeyFromServer);
@@ -357,7 +357,7 @@ public class SetupEncryptionDialogFragment extends DialogFragment implements Inj
             //  - encrypt private key, push key to server, store unencrypted private key in database
 
             try {
-                String publicKey;
+                String publicKeyString;
 
                 // Create public/private key pair
                 KeyPair keyPair = EncryptionUtils.generateKeyPair();
@@ -371,8 +371,13 @@ public class SetupEncryptionDialogFragment extends DialogFragment implements Inj
                 RemoteOperationResult result = operation.execute(user, getContext());
 
                 if (result.isSuccess()) {
+                    publicKeyString = (String) result.getData().get(0);
+
+                    if (!EncryptionUtils.isMatchingKeys(keyPair, publicKeyString)) {
+                        throw new RuntimeException("Wrong CSR returned");
+                    }
+
                     Log_OC.d(TAG, "public key success");
-                    publicKey = (String) result.getData().get(0);
                 } else {
                     keyResult = KEY_FAILED;
                     return "";
@@ -391,10 +396,14 @@ public class SetupEncryptionDialogFragment extends DialogFragment implements Inj
                 if (storePrivateKeyResult.isSuccess()) {
                     Log_OC.d(TAG, "private key success");
 
-                    arbitraryDataProvider.storeOrUpdateKeyValue(user.getAccountName(), EncryptionUtils.PRIVATE_KEY,
+                    arbitraryDataProvider.storeOrUpdateKeyValue(user.getAccountName(),
+                                                                EncryptionUtils.PRIVATE_KEY,
                                                                 privateKeyString);
-                    arbitraryDataProvider.storeOrUpdateKeyValue(user.getAccountName(), EncryptionUtils.PUBLIC_KEY, publicKey);
-                    arbitraryDataProvider.storeOrUpdateKeyValue(user.getAccountName(), EncryptionUtils.MNEMONIC,
+                    arbitraryDataProvider.storeOrUpdateKeyValue(user.getAccountName(),
+                                                                EncryptionUtils.PUBLIC_KEY,
+                                                                publicKeyString);
+                    arbitraryDataProvider.storeOrUpdateKeyValue(user.getAccountName(),
+                                                                EncryptionUtils.MNEMONIC,
                                                                 generateMnemonicString(true));
 
                     keyResult = KEY_CREATED;

app/src/main/java/com/owncloud/android/utils/CsrHelper.java

@@ -20,6 +20,8 @@ import org.bouncycastle.pkcs.jcajce.JcaPKCS10CertificationRequestBuilder;
 import java.io.IOException;
 import java.security.KeyPair;
 
+import androidx.annotation.VisibleForTesting;
+
 /**
  * copied & modified from:
  * https://github.com/awslabs/aws-sdk-android-samples/blob/master/CreateIotCertWithCSR/src/com/amazonaws/demo/csrcert/CsrHelper.java
@@ -55,13 +57,14 @@ public final class CsrHelper {
      * Create the certificate signing request (CSR) from private and public keys
      *
      * @param keyPair the KeyPair with private and public keys
-     * @param userId userId of CSR owner
+     * @param userId  userId of CSR owner
      * @return PKCS10CertificationRequest with the certificate signing request (CSR) data
-     * @throws IOException thrown if key cannot be created
+     * @throws IOException               thrown if key cannot be created
      * @throws OperatorCreationException thrown if contentSigner cannot be build
      */
-    private static PKCS10CertificationRequest generateCSR(KeyPair keyPair, String userId) throws IOException,
-    OperatorCreationException {
+    @VisibleForTesting
+    public static PKCS10CertificationRequest generateCSR(KeyPair keyPair, String userId) throws IOException,
+        OperatorCreationException {
         String principal = "CN=" + userId;
         AsymmetricKeyParameter privateKey = PrivateKeyFactory.createKey(keyPair.getPrivate().getEncoded());
         AlgorithmIdentifier signatureAlgorithm = new DefaultSignatureAlgorithmIdentifierFinder().find("SHA1WITHRSA");

app/src/main/java/com/owncloud/android/utils/EncryptionUtils.java

@@ -54,6 +54,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.RandomAccessFile;
+import java.math.BigInteger;
 import java.nio.charset.StandardCharsets;
 import java.security.InvalidAlgorithmParameterException;
 import java.security.InvalidKeyException;
@@ -69,6 +70,8 @@ import java.security.SecureRandom;
 import java.security.cert.CertificateException;
 import java.security.cert.CertificateFactory;
 import java.security.cert.X509Certificate;
+import java.security.interfaces.RSAPrivateCrtKey;
+import java.security.interfaces.RSAPublicKey;
 import java.security.spec.InvalidKeySpecException;
 import java.security.spec.KeySpec;
 import java.security.spec.PKCS8EncodedKeySpec;
@@ -852,10 +855,33 @@ public final class EncryptionUtils {
         }
     }
 
+    public static RSAPublicKey convertPublicKeyFromString(String string) throws CertificateException {
+        String trimmedCert = string.replace("-----BEGIN CERTIFICATE-----\n", "")
+            .replace("-----END CERTIFICATE-----\n", "");
+        byte[] encodedCert = trimmedCert.getBytes(StandardCharsets.UTF_8);
+        byte[] decodedCert = org.apache.commons.codec.binary.Base64.decodeBase64(encodedCert);
+
+        CertificateFactory certFactory = CertificateFactory.getInstance("X.509");
+        InputStream in = new ByteArrayInputStream(decodedCert);
+        X509Certificate certificate = (X509Certificate) certFactory.generateCertificate(in);
+        return (RSAPublicKey) certificate.getPublicKey();
+    }
+
     public static void removeE2E(ArbitraryDataProvider arbitraryDataProvider, User user) {
         // delete stored E2E keys and mnemonic
         arbitraryDataProvider.deleteKeyForAccount(user.getAccountName(), EncryptionUtils.PRIVATE_KEY);
         arbitraryDataProvider.deleteKeyForAccount(user.getAccountName(), EncryptionUtils.PUBLIC_KEY);
         arbitraryDataProvider.deleteKeyForAccount(user.getAccountName(), EncryptionUtils.MNEMONIC);
     }
+
+    public static boolean isMatchingKeys(KeyPair keyPair, String publicKeyString) throws CertificateException {
+        // check key
+        RSAPrivateCrtKey privateKey = (RSAPrivateCrtKey) keyPair.getPrivate();
+        RSAPublicKey publicKey = EncryptionUtils.convertPublicKeyFromString(publicKeyString);
+
+        BigInteger modulusPublic = publicKey.getModulus();
+        BigInteger modulusPrivate = privateKey.getModulus();
+
+        return modulusPrivate.compareTo(modulusPublic) == 0;
+    }
 }