From 68a6f9f77490f68768fe8164d718430569a9934e Mon Sep 17 00:00:00 2001 From: alperozturk Date: Fri, 23 Aug 2024 15:26:26 +0200 Subject: [PATCH] refactor Signed-off-by: alperozturk --- .../android/utils/EncryptionTestUtils.kt | 2 +- .../android/utils/EncryptionUtilsV2IT.kt | 42 ++--- .../extensions/DecryptedUserExtensions.kt | 22 +++ .../java/com/owncloud/android/MainApp.java | 4 + .../e2e/v2/decrypted/DecryptedUser.kt | 3 +- .../CreateShareWithShareeOperation.java | 8 +- .../android/utils/EncryptionUtils.java | 2 +- .../android/utils/EncryptionUtilsV2.kt | 146 +++++++++--------- app/src/main/res/values/strings.xml | 4 + 9 files changed, 138 insertions(+), 95 deletions(-) create mode 100644 app/src/main/java/com/nextcloud/utils/extensions/DecryptedUserExtensions.kt diff --git a/app/src/androidTest/java/com/owncloud/android/utils/EncryptionTestUtils.kt b/app/src/androidTest/java/com/owncloud/android/utils/EncryptionTestUtils.kt index 8361e1e09dcf..34d6621bed3a 100644 --- a/app/src/androidTest/java/com/owncloud/android/utils/EncryptionTestUtils.kt +++ b/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( diff --git a/app/src/androidTest/java/com/owncloud/android/utils/EncryptionUtilsV2IT.kt b/app/src/androidTest/java/com/owncloud/android/utils/EncryptionUtilsV2IT.kt index a5be7eaba198..41105e97b77a 100644 --- a/app/src/androidTest/java/com/owncloud/android/utils/EncryptionUtilsV2IT.kt +++ b/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( diff --git a/app/src/main/java/com/nextcloud/utils/extensions/DecryptedUserExtensions.kt b/app/src/main/java/com/nextcloud/utils/extensions/DecryptedUserExtensions.kt new file mode 100644 index 000000000000..ba4830681d44 --- /dev/null +++ b/app/src/main/java/com/nextcloud/utils/extensions/DecryptedUserExtensions.kt @@ -0,0 +1,22 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2024 Alper Ozturk + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +package com.nextcloud.utils.extensions + +import com.owncloud.android.datamodel.e2e.v2.decrypted.DecryptedUser + +fun List.findMetadataKeyByUserId(userId: String): String? { + var result: String? = null + + for (decryptedUser in this) { + if (decryptedUser != null && decryptedUser.userId == userId) { + result = decryptedUser.decryptedMetadataKey + } + } + + return result +} diff --git a/app/src/main/java/com/owncloud/android/MainApp.java b/app/src/main/java/com/owncloud/android/MainApp.java index da658edd5c12..9d8507b7b4b0 100644 --- a/app/src/main/java/com/owncloud/android/MainApp.java +++ b/app/src/main/java/com/owncloud/android/MainApp.java @@ -784,6 +784,10 @@ public static String getUserAgent() { 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); diff --git a/app/src/main/java/com/owncloud/android/datamodel/e2e/v2/decrypted/DecryptedUser.kt b/app/src/main/java/com/owncloud/android/datamodel/e2e/v2/decrypted/DecryptedUser.kt index c30b45b9c41f..fe6210442adf 100644 --- a/app/src/main/java/com/owncloud/android/datamodel/e2e/v2/decrypted/DecryptedUser.kt +++ b/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? ) diff --git a/app/src/main/java/com/owncloud/android/operations/CreateShareWithShareeOperation.java b/app/src/main/java/com/owncloud/android/operations/CreateShareWithShareeOperation.java index 59461d031328..7de040d6fad7 100644 --- a/app/src/main/java/com/owncloud/android/operations/CreateShareWithShareeOperation.java +++ b/app/src/main/java/com/owncloud/android/operations/CreateShareWithShareeOperation.java @@ -17,6 +17,7 @@ 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 @@ protected RemoteOperationResult run(OwnCloudClient client) { 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 @@ protected RemoteOperationResult run(OwnCloudClient client) { // 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); diff --git a/app/src/main/java/com/owncloud/android/utils/EncryptionUtils.java b/app/src/main/java/com/owncloud/android/utils/EncryptionUtils.java index 4ceb2847a9cb..e22eef012c55 100644 --- a/app/src/main/java/com/owncloud/android/utils/EncryptionUtils.java +++ b/app/src/main/java/com/owncloud/android/utils/EncryptionUtils.java @@ -1439,7 +1439,7 @@ public static Pair retrieveMetadata(OCFile 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) { diff --git a/app/src/main/java/com/owncloud/android/utils/EncryptionUtilsV2.kt b/app/src/main/java/com/owncloud/android/utils/EncryptionUtilsV2.kt index 042f4251fac9..60ba17da4a98 100644 --- a/app/src/main/java/com/owncloud/android/utils/EncryptionUtilsV2.kt +++ b/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 { 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): 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): 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): 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 } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 065c2e955152..a10c3fcce1ac 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -1078,6 +1078,10 @@ Please update the Android System WebView app for a login Update + Counter is too old + Hash not found + Signature does not match + The link to your %1$s web interface when you open it in the browser. Delayed due to too many wrong attempts Create