Merge pull request #13455 from nextcloud/refactor/general-code-cleanup

Refactor - Utils Code Cleanup

app/src/androidTest/java/com/owncloud/android/utils/EncryptionTestUtils.kt

@@ -118,7 +118,7 @@ nDO4ew==
         )
 
         val users = mutableListOf(
-            DecryptedUser(userId, cert)
+            DecryptedUser(userId, cert, null)
         )
 
         // val filedrop = mutableMapOf(

app/src/androidTest/java/com/owncloud/android/utils/EncryptionUtilsV2IT.kt

@@ -10,6 +10,7 @@ package com.owncloud.android.utils
 import com.google.gson.reflect.TypeToken
 import com.nextcloud.client.account.MockUser
 import com.nextcloud.common.User
+import com.nextcloud.utils.extensions.findMetadataKeyByUserId
 import com.owncloud.android.EncryptionIT
 import com.owncloud.android.datamodel.OCFile
 import com.owncloud.android.datamodel.e2e.v1.decrypted.Data
@@ -221,7 +222,7 @@ class EncryptionUtilsV2IT : EncryptionIT() {
         val metadataKeyBase64 = EncryptionUtils.generateKeyString()
         val metadataKey = EncryptionUtils.decodeStringToBase64Bytes(metadataKeyBase64)
 
-        val user = DecryptedUser("t1", encryptionTestUtils.t1PublicKey)
+        val user = DecryptedUser("t1", encryptionTestUtils.t1PublicKey, null)
 
         val encryptedUser = encryptionUtilsV2.encryptUser(user, metadataKey)
         assertNotEquals(encryptedUser.encryptedMetadataKey, metadataKeyBase64)
@@ -274,6 +275,11 @@ class EncryptionUtilsV2IT : EncryptionIT() {
             arbitraryDataProvider
         )
 
+        // V1 doesn't have decryptedMetadataKey so that we can ignore it for comparison
+        for (user in decrypted.users) {
+            user.decryptedMetadataKey = null
+        }
+
         assertEquals(metadataFile, decrypted)
     }
 
@@ -489,7 +495,7 @@ class EncryptionUtilsV2IT : EncryptionIT() {
 
         var metadataFile = generateDecryptedFolderMetadataFile(enc1, enc1Cert)
 
-        metadataFile = encryptionUtilsV2.addShareeToMetadata(metadataFile, enc2.accountName, enc2Cert)
+        metadataFile = encryptionUtilsV2.addShareeToMetadata(metadataFile, enc2.accountName, enc2Cert, null)
 
         val encryptedMetadataFile = encryptionUtilsV2.encryptFolderMetadataFile(
             metadataFile,
@@ -541,7 +547,12 @@ class EncryptionUtilsV2IT : EncryptionIT() {
         val enc1 = MockUser("enc1", "Nextcloud")
         val enc2 = MockUser("enc2", "Nextcloud")
         var metadataFile = generateDecryptedFolderMetadataFile(enc1, enc1Cert)
-        metadataFile = encryptionUtilsV2.addShareeToMetadata(metadataFile, enc2.accountName, enc2Cert)
+        metadataFile = encryptionUtilsV2.addShareeToMetadata(
+            metadataFile,
+            enc2.accountName,
+            enc2Cert,
+            metadataFile.users.findMetadataKeyByUserId(enc2.accountName)
+        )
 
         assertEquals(2, metadataFile.users.size)
 
@@ -586,7 +597,7 @@ class EncryptionUtilsV2IT : EncryptionIT() {
         )
 
         val users = mutableListOf(
-            DecryptedUser(user.accountName, cert)
+            DecryptedUser(user.accountName, cert, null)
         )
 
         metadata.keyChecksums.add(encryptionUtilsV2.hashMetadataKey(metadata.metadataKey))
@@ -734,8 +745,6 @@ class EncryptionUtilsV2IT : EncryptionIT() {
                 |Rei/RGBQ==","userId": "john"}],"version": "2"}
             """.trimMargin()
 
-        val base64Metadata = EncryptionUtils.encodeStringToBase64String(metadata)
-
         val privateKey = EncryptionUtils.PEMtoPrivateKey(encryptionTestUtils.t1PrivateKey)
         val certificateT1 = EncryptionUtils.convertCertFromString(encryptionTestUtils.t1PublicKey)
         val certificateEnc2 = EncryptionUtils.convertCertFromString(enc2Cert)
@@ -746,23 +755,18 @@ class EncryptionUtilsV2IT : EncryptionIT() {
             metadata
         )
 
-        val base64Ans = encryptionUtilsV2.extractSignedString(signed)
-
-        // verify
         val certs = listOf(
             certificateEnc2,
             certificateT1
         )
-        assertTrue(encryptionUtilsV2.verifySignedMessage(signed, certs))
-        assertTrue(encryptionUtilsV2.verifySignedMessage(base64Ans, base64Metadata, certs))
+
+        assertTrue(encryptionUtilsV2.verifySignedData(signed, certs))
     }
 
     @Throws(Throwable::class)
     @Test
     fun sign() {
         val sut = "randomstring123"
-        val json = "randomstring123"
-        val jsonBase64 = EncryptionUtils.encodeStringToBase64String(json)
 
         val privateKey = EncryptionUtils.PEMtoPrivateKey(encryptionTestUtils.t1PrivateKey)
         val certificate = EncryptionUtils.convertCertFromString(encryptionTestUtils.t1PublicKey)
@@ -773,15 +777,12 @@ class EncryptionUtilsV2IT : EncryptionIT() {
             sut
         )
 
-        val base64Ans = encryptionUtilsV2.extractSignedString(signed)
-
-        // verify
         val certs = listOf(
             EncryptionUtils.convertCertFromString(enc2Cert),
             certificate
         )
-        assertTrue(encryptionUtilsV2.verifySignedMessage(signed, certs))
-        assertTrue(encryptionUtilsV2.verifySignedMessage(base64Ans, jsonBase64, certs))
+
+        assertTrue(encryptionUtilsV2.verifySignedData(signed, certs))
     }
 
     @Test
@@ -857,6 +858,11 @@ class EncryptionUtilsV2IT : EncryptionIT() {
             arbitraryDataProvider
         )
 
+        // V1 doesn't have decryptedMetadataKey so that we can ignore it for comparison
+        for (user in decryptedFolderMetadata2.users) {
+            user.decryptedMetadataKey = null
+        }
+
         // compare
         assertTrue(
             EncryptionTestIT.compareJsonStrings(

app/src/main/java/com/nextcloud/utils/extensions/DecryptedUserExtensions.kt

@@ -0,0 +1,22 @@
+/*
+ * Nextcloud - Android Client
+ *
+ * SPDX-FileCopyrightText: 2024 Alper Ozturk <alper.ozturk@nextcloud.com>
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+package com.nextcloud.utils.extensions
+
+import com.owncloud.android.datamodel.e2e.v2.decrypted.DecryptedUser
+
+fun List<DecryptedUser?>.findMetadataKeyByUserId(userId: String): String? {
+    var result: String? = null
+
+    for (decryptedUser in this) {
+        if (decryptedUser != null && decryptedUser.userId == userId) {
+            result = decryptedUser.decryptedMetadataKey
+        }
+    }
+
+    return result
+}

app/src/main/java/com/owncloud/android/MainApp.java

@@ -784,6 +784,10 @@ public class MainApp extends Application implements HasAndroidInjector {
         return getUserAgent(R.string.nextcloud_user_agent);
     }
 
+    public static void showMessage(int messageId) {
+        ContextExtensionsKt.showToast(getAppContext(), messageId);
+    }
+
     // user agent
     private static String getUserAgent(@StringRes int agent) {
         String appString = string(agent);

app/src/main/java/com/owncloud/android/datamodel/e2e/v2/decrypted/DecryptedUser.kt

@@ -9,5 +9,6 @@ package com.owncloud.android.datamodel.e2e.v2.decrypted
 
 data class DecryptedUser(
     val userId: String,
-    val certificate: String
+    val certificate: String,
+    var decryptedMetadataKey: String?
 )

app/src/main/java/com/owncloud/android/operations/CreateShareWithShareeOperation.java

@@ -17,6 +17,7 @@ import com.nextcloud.client.account.User;
 import com.nextcloud.client.network.ClientFactory;
 import com.nextcloud.client.network.ClientFactoryImpl;
 import com.nextcloud.common.NextcloudClient;
+import com.nextcloud.utils.extensions.DecryptedUserExtensionsKt;
 import com.owncloud.android.R;
 import com.owncloud.android.datamodel.ArbitraryDataProvider;
 import com.owncloud.android.datamodel.FileDataStorageManager;
@@ -183,7 +184,7 @@ public class CreateShareWithShareeOperation extends SyncOperation {
             if (metadata == null) {
                 String cert = EncryptionUtils.retrievePublicKeyForUser(user, context);
                 metadata = new EncryptionUtilsV2().createDecryptedFolderMetadataFile();
-                metadata.getUsers().add(new DecryptedUser(client.getUserId(), cert));
+                metadata.getUsers().add(new DecryptedUser(client.getUserId(), cert, null));
 
                 metadataExists = false;
             } else {
@@ -194,9 +195,12 @@ public class CreateShareWithShareeOperation extends SyncOperation {
 
             // add sharee to metadata
             String publicKey = EncryptionUtils.getPublicKey(user, shareeName, arbitraryDataProvider);
+
+            String decryptedMetadataKey = DecryptedUserExtensionsKt.findMetadataKeyByUserId(metadata.getUsers(), shareeName);
             DecryptedFolderMetadataFile newMetadata = encryptionUtilsV2.addShareeToMetadata(metadata,
                                                                                             shareeName,
-                                                                                            publicKey);
+                                                                                            publicKey,
+                                                                                            decryptedMetadataKey);
 
             // upload metadata
             metadata.getMetadata().setCounter(newCounter);

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

@@ -1439,7 +1439,7 @@ public final class EncryptionUtils {
                                                        new ArrayList<>(),
                                                        new HashMap<>(),
                                                        E2EVersion.V2_0.getValue());
-            metadata.getUsers().add(new DecryptedUser(client.getUserId(), publicKey));
+            metadata.getUsers().add(new DecryptedUser(client.getUserId(), publicKey, null));
             byte[] metadataKey = EncryptionUtils.generateKey();
 
             if (metadataKey == null) {

app/src/main/java/com/owncloud/android/utils/EncryptionUtilsV2.kt

@@ -12,6 +12,8 @@ import android.content.Context
 import androidx.annotation.VisibleForTesting
 import com.google.gson.reflect.TypeToken
 import com.nextcloud.client.account.User
+import com.owncloud.android.MainApp
+import com.owncloud.android.R
 import com.owncloud.android.datamodel.ArbitraryDataProvider
 import com.owncloud.android.datamodel.ArbitraryDataProviderImpl
 import com.owncloud.android.datamodel.FileDataStorageManager
@@ -398,7 +400,8 @@ class EncryptionUtilsV2 {
     fun transformUser(user: EncryptedUser): DecryptedUser {
         return DecryptedUser(
             user.userId,
-            user.certificate
+            user.certificate,
+            user.encryptedMetadataKey
         )
     }
 
@@ -454,9 +457,10 @@ class EncryptionUtilsV2 {
     fun addShareeToMetadata(
         metadataFile: DecryptedFolderMetadataFile,
         userId: String,
-        cert: String
+        cert: String,
+        decryptedMetadataKey: String?
     ): DecryptedFolderMetadataFile {
-        metadataFile.users.add(DecryptedUser(userId, cert))
+        metadataFile.users.add(DecryptedUser(userId, cert, decryptedMetadataKey))
         metadataFile.metadata.metadataKey = EncryptionUtils.generateKey()
         metadataFile.metadata.keyChecksums.add(hashMetadataKey(metadataFile.metadata.metadataKey))
 
@@ -553,7 +557,6 @@ class EncryptionUtilsV2 {
         context: Context
     ): Pair<Boolean, DecryptedFolderMetadataFile> {
         val getMetadataOperationResult = GetMetadataRemoteOperation(folder.localId).execute(client)
-
         return if (getMetadataOperationResult.isSuccess) {
             // decrypt metadata
             val metadataResponse = getMetadataOperationResult.resultData
@@ -580,7 +583,7 @@ class EncryptionUtilsV2 {
                 val publicKey: String = arbitraryDataProvider.getValue(user.accountName, EncryptionUtils.PUBLIC_KEY)
 
                 createDecryptedFolderMetadataFile().apply {
-                    users = mutableListOf(DecryptedUser(client.userId, publicKey))
+                    users = mutableListOf(DecryptedUser(client.userId, publicKey, null))
                 }
             }
 
@@ -829,7 +832,7 @@ class EncryptionUtilsV2 {
 
         // upon migration there can only be one user, as there is no sharing yet in place
         val users = if (storageManager.getFileById(folder.parentId)?.isEncrypted == false) {
-            mutableListOf(DecryptedUser(userId, cert))
+            mutableListOf(DecryptedUser(userId, cert, null))
         } else {
             mutableListOf()
         }
@@ -943,61 +946,63 @@ class EncryptionUtilsV2 {
         }
     }
 
-    @Throws(IllegalStateException::class)
-    @Suppress("ThrowsCount")
-    @VisibleForTesting
+    @Suppress("ReturnCount")
     fun verifyMetadata(
         encryptedFolderMetadataFile: EncryptedFolderMetadataFile,
         decryptedFolderMetadataFile: DecryptedFolderMetadataFile,
         oldCounter: Long,
-        // base 64 encoded BER
-        ans: String
+        signature: String
     ) {
-        // check counter
         if (decryptedFolderMetadataFile.metadata.counter < oldCounter) {
-            throw IllegalStateException("Counter is too old")
+            MainApp.showMessage(R.string.e2e_counter_too_old)
+            return
         }
 
-        // check signature
-        val json = EncryptionUtils.serializeJSON(encryptedFolderMetadataFile, true)
+        val message = EncryptionUtils.serializeJSON(encryptedFolderMetadataFile, true)
         val certs = decryptedFolderMetadataFile.users.map { EncryptionUtils.convertCertFromString(it.certificate) }
+        val signedData = getSignedData(signature, message)
 
-        val base64 = EncryptionUtils.encodeStringToBase64String(json)
-
-        // if (!verifySignedMessage(ans, base64, certs)) {
-        //     throw IllegalStateException("Signature does not match")
-        // }
+        if (!verifySignedData(signedData, certs)) {
+            MainApp.showMessage(R.string.e2e_signature_does_not_match)
+            return
+        }
 
         val hashedMetadataKey = hashMetadataKey(decryptedFolderMetadataFile.metadata.metadataKey)
         if (!decryptedFolderMetadataFile.metadata.keyChecksums.contains(hashedMetadataKey)) {
-            throw IllegalStateException("Hash not found")
-            // TODO E2E: fake this to present problem to user
+            MainApp.showMessage(R.string.e2e_hash_not_found)
+            return
         }
-
-        // TODO E2E: check certs
     }
 
-    fun createDecryptedFolderMetadataFile(): DecryptedFolderMetadataFile {
-        val metadata = DecryptedMetadata().apply {
-            keyChecksums.add(hashMetadataKey(metadataKey))
-        }
+    fun getSignedData(base64encodedSignature: String, message: String): CMSSignedData {
+        val signature = EncryptionUtils.decodeStringToBase64Bytes(base64encodedSignature)
+        val asn1Signature = ASN1Sequence.fromByteArray(signature)
+        val contentInfo = ContentInfo.getInstance(asn1Signature)
 
-        return DecryptedFolderMetadataFile(metadata)
+        val encodedMessage = EncryptionUtils.encodeStringToBase64String(message)
+        val messageData = encodedMessage.toByteArray()
+        val cmsProcessableByteArray = CMSProcessableByteArray(messageData)
+
+        return CMSSignedData(cmsProcessableByteArray, contentInfo)
     }
 
-    /**
-     * SHA-256 hash of metadata-key
-     */
-    @Suppress("MagicNumber")
-    fun hashMetadataKey(metadataKey: ByteArray): String {
-        val bytes = MessageDigest
-            .getInstance("SHA-256")
-            .digest(metadataKey)
+    fun verifySignedData(data: CMSSignedData, certs: List<X509Certificate>): Boolean {
+        val signer: SignerInformation = data.signerInfos.signers.iterator().next() as SignerInformation
 
-        return BigInteger(1, bytes).toString(16).padStart(32, '0')
+        certs.forEach {
+            try {
+                if (signer.verify(JcaSimpleSignerInfoVerifierBuilder().build(it))) {
+                    return true
+                }
+            } catch (e: java.lang.Exception) {
+                Log_OC.e(TAG, "Error caught at verifySignedData: $e")
+            }
+        }
+
+        return false
     }
 
-    fun signMessage(cert: X509Certificate, key: PrivateKey, data: ByteArray): CMSSignedData {
+    private fun signMessage(cert: X509Certificate, key: PrivateKey, data: ByteArray): CMSSignedData {
         val content = CMSProcessableByteArray(data)
         val certs = JcaCertStore(listOf(cert))
 
@@ -1021,7 +1026,11 @@ class EncryptionUtilsV2 {
      * Sign the data with key, embed the certificate associated within the CMSSignedData
      * detached data not possible, as to restore asn.1
      */
-    fun signMessage(cert: X509Certificate, key: PrivateKey, message: EncryptedFolderMetadataFile): CMSSignedData {
+    private fun signMessage(
+        cert: X509Certificate,
+        key: PrivateKey,
+        message: EncryptedFolderMetadataFile
+    ): CMSSignedData {
         val json = EncryptionUtils.serializeJSON(message, true)
         val base64 = EncryptionUtils.encodeStringToBase64String(json)
         val data = base64.toByteArray()
@@ -1041,6 +1050,26 @@ class EncryptionUtilsV2 {
         return EncryptionUtils.encodeBytesToBase64String(ans)
     }
 
+    fun createDecryptedFolderMetadataFile(): DecryptedFolderMetadataFile {
+        val metadata = DecryptedMetadata().apply {
+            keyChecksums.add(hashMetadataKey(metadataKey))
+        }
+
+        return DecryptedFolderMetadataFile(metadata)
+    }
+
+    /**
+     * SHA-256 hash of metadata-key
+     */
+    @Suppress("MagicNumber")
+    fun hashMetadataKey(metadataKey: ByteArray): String {
+        val bytes = MessageDigest
+            .getInstance("SHA-256")
+            .digest(metadataKey)
+
+        return BigInteger(1, bytes).toString(16).padStart(32, '0')
+    }
+
     fun getMessageSignature(cert: String, privateKey: String, metadataFile: EncryptedFolderMetadataFile): String {
         return getMessageSignature(
             EncryptionUtils.convertCertFromString(cert),
@@ -1049,7 +1078,11 @@ class EncryptionUtilsV2 {
         )
     }
 
-    fun getMessageSignature(cert: X509Certificate, key: PrivateKey, message: EncryptedFolderMetadataFile): String {
+    private fun getMessageSignature(
+        cert: X509Certificate,
+        key: PrivateKey,
+        message: EncryptedFolderMetadataFile
+    ): String {
         val signedMessage = signMessage(cert, key, message)
         return extractSignedString(signedMessage)
     }
@@ -1059,37 +1092,6 @@ class EncryptionUtilsV2 {
         return extractSignedString(signedMessage)
     }
 
-    /**
-     * Verify the signature but does not use the certificate in the signed object
-     */
-    fun verifySignedMessage(data: CMSSignedData, certs: List<X509Certificate>): Boolean {
-        val signer: SignerInformation = data.signerInfos.signers.iterator().next() as SignerInformation
-
-        certs.forEach {
-            try {
-                if (signer.verify(JcaSimpleSignerInfoVerifierBuilder().build(it))) {
-                    return true
-                }
-            } catch (e: java.lang.Exception) {
-                Log_OC.e("Encryption", "error", e)
-            }
-        }
-
-        return false
-    }
-
-    /**
-     * Verify the signature but does not use the certificate in the signed object
-     */
-    fun verifySignedMessage(base64encodedAns: String, originalMessage: String, certs: List<X509Certificate>): Boolean {
-        val ans = EncryptionUtils.decodeStringToBase64Bytes(base64encodedAns)
-        val contentInfo = ContentInfo.getInstance(ASN1Sequence.fromByteArray(ans))
-        val content = CMSProcessableByteArray(originalMessage.toByteArray())
-        val sig = CMSSignedData(content, contentInfo)
-
-        return verifySignedMessage(sig, certs)
-    }
-
     companion object {
         private val TAG = EncryptionUtils::class.java.simpleName
     }

app/src/main/res/values/strings.xml

@@ -1078,6 +1078,10 @@
     <string name="webview_version_check_alert_dialog_message">Please update the Android System WebView app for a login</string>
     <string name="webview_version_check_alert_dialog_positive_button_title">Update</string>
 
+    <string name="e2e_counter_too_old">Counter is too old</string>
+    <string name="e2e_hash_not_found">Hash not found</string>
+    <string name="e2e_signature_does_not_match">Signature does not match</string>
+
     <string name="login_url_helper_text">The link to your %1$s web interface when you open it in the browser.</string>
     <string name="brute_force_delay">Delayed due to too many wrong attempts</string>
     <string name="create">Create</string>