From 771272d9d4d63def5c4ab83c674b124985ed9d5d Mon Sep 17 00:00:00 2001 From: Li Wei Date: Sat, 14 Nov 2026 23:39:25 +0800 Subject: [PATCH] fix(e2ee): cache server public key during folder browsing Reuse successful server public key responses for ten minutes, scoped to the server URL and current user certificate. Keep certificate verification on every entry and merge concurrent cache misses. Assisted-by: Codex:GPT-6 Signed-off-by: Li Wei --- .../owncloud/android/EncryptedFoldersIT.kt | 12 +- .../java/com/nextcloud/client/di/AppModule.kt | 9 +- .../nextcloud/utils/e2ee/E2EEKeyInspector.kt | 20 ++- .../utils/e2ee/E2EEServerKeyCache.kt | 43 ++++++ .../e2ee/E2EEKeyInspectorServerKeyTest.kt | 68 +++++++++ .../utils/e2ee/E2EEServerKeyCacheTest.kt | 134 ++++++++++++++++++ 6 files changed, 276 insertions(+), 10 deletions(-) create mode 100644 app/src/main/java/com/nextcloud/utils/e2ee/E2EEServerKeyCache.kt create mode 100644 app/src/test/java/com/nextcloud/utils/e2ee/E2EEKeyInspectorServerKeyTest.kt create mode 100644 app/src/test/java/com/nextcloud/utils/e2ee/E2EEServerKeyCacheTest.kt diff --git a/app/src/androidTest/java/com/owncloud/android/EncryptedFoldersIT.kt b/app/src/androidTest/java/com/owncloud/android/EncryptedFoldersIT.kt index 0a7d511741d0..0d66fff3b402 100644 --- a/app/src/androidTest/java/com/owncloud/android/EncryptedFoldersIT.kt +++ b/app/src/androidTest/java/com/owncloud/android/EncryptedFoldersIT.kt @@ -9,11 +9,14 @@ package com.owncloud.android import com.nextcloud.client.account.UserAccountManager import com.nextcloud.client.account.UserAccountManagerImpl +import com.nextcloud.client.core.ClockImpl import com.nextcloud.client.database.entity.FileEntity import com.nextcloud.client.network.NetworkModule +import com.nextcloud.repository.ClientRepository import com.nextcloud.test.SinceServer import com.nextcloud.utils.e2ee.E2EEActionResolver import com.nextcloud.utils.e2ee.E2EEKeyInspector +import com.nextcloud.utils.e2ee.E2EEServerKeyCache import com.owncloud.android.datamodel.OCFile import com.owncloud.android.lib.resources.e2ee.ToggleEncryptionRemoteOperation import com.owncloud.android.lib.resources.status.GetCapabilitiesRemoteOperation @@ -33,6 +36,7 @@ import org.junit.Assert.assertNotNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test +import org.mockito.kotlin.mock @SinceServer(majorVersion = 30) open class EncryptedFoldersIT : AbstractOnServerIT() { @@ -61,12 +65,18 @@ open class EncryptedFoldersIT : AbstractOnServerIT() { init { val accountManager: UserAccountManager = UserAccountManagerImpl.fromContext(targetContext) + val clientRepository: ClientRepository = mock { + onBlocking { getOwncloudClient() }.thenReturn(client) + onBlocking { getNextcloudClient() }.thenReturn(nextcloudClient) + } val inspector = E2EEKeyInspector( targetContext, storageManager, CertificateValidator(), arbitraryDataProvider, - accountManager + accountManager, + clientRepository, + E2EEServerKeyCache(ClockImpl()) ) e2eeActionResolver = E2EEActionResolver( storageManager, diff --git a/app/src/main/java/com/nextcloud/client/di/AppModule.kt b/app/src/main/java/com/nextcloud/client/di/AppModule.kt index 8154074c608d..c3f5dd10586b 100644 --- a/app/src/main/java/com/nextcloud/client/di/AppModule.kt +++ b/app/src/main/java/com/nextcloud/client/di/AppModule.kt @@ -46,6 +46,7 @@ import com.nextcloud.repository.ClientRepository import com.nextcloud.repository.RemoteClientRepository import com.nextcloud.utils.e2ee.E2EEActionResolver import com.nextcloud.utils.e2ee.E2EEKeyInspector +import com.nextcloud.utils.e2ee.E2EEServerKeyCache import com.nextcloud.utils.thumbnail.FolderThumbnailGenerator import com.owncloud.android.authentication.PassCodeManager import com.owncloud.android.datamodel.ArbitraryDataProvider @@ -239,13 +240,17 @@ internal class AppModule { fileDataStorageManager: FileDataStorageManager, certificateValidator: CertificateValidator, arbitraryDataProvider: ArbitraryDataProvider, - accountManager: UserAccountManager + accountManager: UserAccountManager, + clientRepository: ClientRepository, + serverKeyCache: E2EEServerKeyCache ): E2EEKeyInspector = E2EEKeyInspector( context, fileDataStorageManager, certificateValidator, arbitraryDataProvider, - accountManager + accountManager, + clientRepository, + serverKeyCache ) @Provides diff --git a/app/src/main/java/com/nextcloud/utils/e2ee/E2EEKeyInspector.kt b/app/src/main/java/com/nextcloud/utils/e2ee/E2EEKeyInspector.kt index 18b4c98d4542..9adee7357be8 100644 --- a/app/src/main/java/com/nextcloud/utils/e2ee/E2EEKeyInspector.kt +++ b/app/src/main/java/com/nextcloud/utils/e2ee/E2EEKeyInspector.kt @@ -10,12 +10,12 @@ package com.nextcloud.utils.e2ee import android.content.Context import com.google.gson.reflect.TypeToken import com.nextcloud.client.account.UserAccountManager +import com.nextcloud.repository.ClientRepository import com.owncloud.android.datamodel.ArbitraryDataProvider import com.owncloud.android.datamodel.FileDataStorageManager import com.owncloud.android.datamodel.OCFile import com.owncloud.android.datamodel.e2e.v1.encrypted.EncryptedFolderMetadataFileV1 import com.owncloud.android.lib.common.OwnCloudClient -import com.owncloud.android.lib.common.OwnCloudClientFactory import com.owncloud.android.lib.common.utils.Log_OC import com.owncloud.android.lib.resources.e2ee.GetMetadataRemoteOperation import com.owncloud.android.lib.resources.status.OCCapability @@ -36,7 +36,9 @@ class E2EEKeyInspector @Inject constructor( private val storageManager: FileDataStorageManager, private val certificateValidator: CertificateValidator, private val arbitraryDataProvider: ArbitraryDataProvider, - private val accountManager: UserAccountManager + private val accountManager: UserAccountManager, + private val clientRepository: ClientRepository, + private val serverKeyCache: E2EEServerKeyCache ) { companion object { private const val TAG = "E2EEKeyInspector" @@ -47,7 +49,8 @@ class E2EEKeyInspector @Inject constructor( arbitraryDataProvider.getValue(accountManager.user, EncryptionUtils.PUBLIC_KEY).isEmpty() suspend fun fetchCapabilities(): Boolean = withContext(Dispatchers.IO) { - val result = GetCapabilitiesOperation(storageManager).execute(context).isSuccess + val client = clientRepository.getOwncloudClient() ?: return@withContext false + val result = GetCapabilitiesOperation(storageManager).execute(client).isSuccess if (result) { Log_OC.i(TAG, "capabilities fetched") } else { @@ -59,9 +62,10 @@ class E2EEKeyInspector @Inject constructor( suspend fun compareWithServerKey(storedPublicKey: String): DownloadKeyResult = withContext(Dispatchers.IO) { Log_OC.d(TAG, "comparing local public key with remote public key") - val user = accountManager.user + val client = clientRepository.getNextcloudClient() + ?: return@withContext DownloadKeyResult.UnexpectedError() - val certificateResult = GetCertificateRemoteOperation().executeNextcloudClient(user, context) + val certificateResult = GetCertificateRemoteOperation().execute(client) if (!certificateResult.isSuccess) { return@withContext if (certificateResult.httpCode == HttpStatus.SC_NOT_FOUND) { DownloadKeyResult.NoServerKey @@ -70,7 +74,9 @@ class E2EEKeyInspector @Inject constructor( } } - val serverPublicKeyResult = GetServerPublicKeyRemoteOperation().executeNextcloudClient(user, context) + val serverPublicKeyResult = serverKeyCache.get(client.baseUri.toString(), certificateResult.resultData) { + GetServerPublicKeyRemoteOperation().execute(client) + } if (!serverPublicKeyResult.isSuccess) return@withContext DownloadKeyResult.ServerPublicKeyUnavailable() val isCertificateValid = certificateValidator.validate( @@ -86,7 +92,7 @@ class E2EEKeyInspector @Inject constructor( withContext(Dispatchers.IO) { Log_OC.d(TAG, "checking folder metadata key") - val client = OwnCloudClientFactory.createOwnCloudClient(accountManager.currentAccount, context) + val client = clientRepository.getOwncloudClient() ?: return@withContext false val metadataResult = GetMetadataRemoteOperation(folder.localId).execute(client) if (!metadataResult.isSuccess) { diff --git a/app/src/main/java/com/nextcloud/utils/e2ee/E2EEServerKeyCache.kt b/app/src/main/java/com/nextcloud/utils/e2ee/E2EEServerKeyCache.kt new file mode 100644 index 000000000000..35ffcf6532aa --- /dev/null +++ b/app/src/main/java/com/nextcloud/utils/e2ee/E2EEServerKeyCache.kt @@ -0,0 +1,43 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +package com.nextcloud.utils.e2ee + +import androidx.annotation.WorkerThread +import com.nextcloud.client.core.Clock +import com.owncloud.android.lib.common.operations.RemoteOperationResult +import java.util.concurrent.TimeUnit +import javax.inject.Inject +import javax.inject.Singleton + +@Singleton +class E2EEServerKeyCache @Inject constructor(private val clock: Clock) { + private val entries = mutableMapOf, Pair>>() + + @WorkerThread + @Synchronized + fun get( + serverUrl: String, + certificate: String, + fetch: () -> RemoteOperationResult + ): RemoteOperationResult { + val now = clock.millisSinceBoot + entries.entries.removeAll { now - it.value.first !in 0 until LIFETIME_MILLIS } + val key = serverUrl to certificate + entries[key]?.let { return it.second } + + return fetch().also { result -> + if (result.isSuccess && !result.resultData.isNullOrEmpty()) { + entries[key] = clock.millisSinceBoot to result + } + } + } + + companion object { + private val LIFETIME_MILLIS = TimeUnit.MINUTES.toMillis(10) + } +} diff --git a/app/src/test/java/com/nextcloud/utils/e2ee/E2EEKeyInspectorServerKeyTest.kt b/app/src/test/java/com/nextcloud/utils/e2ee/E2EEKeyInspectorServerKeyTest.kt new file mode 100644 index 000000000000..48bdf7e2f663 --- /dev/null +++ b/app/src/test/java/com/nextcloud/utils/e2ee/E2EEKeyInspectorServerKeyTest.kt @@ -0,0 +1,68 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +package com.nextcloud.utils.e2ee + +import android.net.Uri +import com.nextcloud.client.core.Clock +import com.nextcloud.common.NextcloudClient +import com.nextcloud.repository.ClientRepository +import com.owncloud.android.lib.common.operations.RemoteOperationResult +import com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode +import com.owncloud.android.lib.resources.users.GetCertificateRemoteOperation +import com.owncloud.android.lib.resources.users.GetServerPublicKeyRemoteOperation +import com.owncloud.android.ui.dialog.setupEncryption.CertificateValidator +import com.owncloud.android.ui.dialog.setupEncryption.model.DownloadKeyResult +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import org.junit.Assert.assertEquals +import org.junit.Test +import org.mockito.Mockito.mockConstruction +import org.mockito.kotlin.doReturn +import org.mockito.kotlin.mock +import org.mockito.kotlin.times +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever + +class E2EEKeyInspectorServerKeyTest { + @Test + fun repeatedEntryFetchesCertificateAndVerifiesItWhileSharingServerKeyAcrossInspectors() = runTest { + withContext(Dispatchers.IO) { + val uri: Uri = mock { on { toString() } doReturn "https://cloud.example" } + val client: NextcloudClient = mock { on { baseUri } doReturn uri } + val repository: ClientRepository = mock { onBlocking { getNextcloudClient() } doReturn client } + val validator: CertificateValidator = mock() + whenever(validator.validate(SERVER_KEY, CERTIFICATE)).thenReturn(true) + val cache = E2EEServerKeyCache(mock()) + val certificateResult = RemoteOperationResult(ResultCode.OK).apply { resultData = CERTIFICATE } + val serverKeyResult = RemoteOperationResult(ResultCode.OK).apply { resultData = SERVER_KEY } + + mockConstruction(GetCertificateRemoteOperation::class.java) { operation, _ -> + whenever(operation.execute(client)).thenReturn(certificateResult) + }.use { certificates -> + mockConstruction(GetServerPublicKeyRemoteOperation::class.java) { operation, _ -> + whenever(operation.execute(client)).thenReturn(serverKeyResult) + }.use { serverKeys -> + repeat(ENTRIES) { + val inspector = E2EEKeyInspector(mock(), mock(), validator, mock(), mock(), repository, cache) + assertEquals(DownloadKeyResult.CompareKeys(true), inspector.compareWithServerKey(CERTIFICATE)) + } + assertEquals(ENTRIES, certificates.constructed().size) + assertEquals(1, serverKeys.constructed().size) + } + } + verify(validator, times(ENTRIES)).validate(SERVER_KEY, CERTIFICATE) + } + } + + companion object { + private const val SERVER_KEY = "server-key" + private const val CERTIFICATE = "certificate" + private const val ENTRIES = 10 + } +} diff --git a/app/src/test/java/com/nextcloud/utils/e2ee/E2EEServerKeyCacheTest.kt b/app/src/test/java/com/nextcloud/utils/e2ee/E2EEServerKeyCacheTest.kt new file mode 100644 index 000000000000..2ab4766a4238 --- /dev/null +++ b/app/src/test/java/com/nextcloud/utils/e2ee/E2EEServerKeyCacheTest.kt @@ -0,0 +1,134 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +package com.nextcloud.utils.e2ee + +import com.nextcloud.client.core.Clock +import com.owncloud.android.lib.common.operations.RemoteOperationResult +import com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode +import org.junit.Assert.assertEquals +import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue +import org.junit.Test +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever +import java.util.concurrent.TimeUnit +import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executors +import java.util.concurrent.atomic.AtomicInteger + +class E2EEServerKeyCacheTest { + private val clock: Clock = mock() + private val cache = E2EEServerKeyCache(clock) + private val success = RemoteOperationResult(ResultCode.OK).apply { resultData = "server-key" } + + @Test + fun repeatedFolderEntryReusesServerKey() { + var requests = 0 + repeat(REPEATED_ENTRIES) { + assertSame( + success, + cache.get(SERVER, CERTIFICATE) { + requests++ + success + } + ) + } + assertEquals(1, requests) + } + + @Test + fun expirationFetchesServerKeyAgain() { + var requests = 0 + cache.get(SERVER, CERTIFICATE) { + requests++ + success + } + whenever(clock.millisSinceBoot).thenReturn(TimeUnit.MINUTES.toMillis(10)) + cache.get(SERVER, CERTIFICATE) { + requests++ + success + } + assertEquals(2, requests) + } + + @Test + fun changedCertificateFetchesServerKeyAgain() { + var requests = 0 + cache.get(SERVER, CERTIFICATE) { + requests++ + success + } + cache.get(SERVER, "renewed-certificate") { + requests++ + success + } + assertEquals(2, requests) + } + + @Test + fun differentServersDoNotShareCachedKey() { + var requests = 0 + cache.get(SERVER, CERTIFICATE) { + requests++ + success + } + cache.get("https://cloud.example:8443", CERTIFICATE) { + requests++ + success + } + cache.get("https://other.example", CERTIFICATE) { + requests++ + success + } + assertEquals(3, requests) + } + + @Test + fun failuresAndEmptyResponsesAreNotCached() { + val failure = RemoteOperationResult(ResultCode.FORBIDDEN) + val empty = RemoteOperationResult(ResultCode.OK).apply { resultData = "" } + assertSame(failure, cache.get(SERVER, CERTIFICATE) { failure }) + assertSame(empty, cache.get(SERVER, CERTIFICATE) { empty }) + assertSame(success, cache.get(SERVER, CERTIFICATE) { success }) + } + + @Test + fun concurrentFolderEntriesShareOneRequest() { + val executor = Executors.newFixedThreadPool(REPEATED_ENTRIES) + val started = CountDownLatch(1) + val response = CountDownLatch(1) + val requests = AtomicInteger() + try { + val results = List(REPEATED_ENTRIES) { + executor.submit> { + cache.get(SERVER, CERTIFICATE) { + requests.incrementAndGet() + started.countDown() + check(response.await(TIMEOUT_SECONDS, TimeUnit.SECONDS)) + success + } + } + } + assertTrue(started.await(TIMEOUT_SECONDS, TimeUnit.SECONDS)) + assertEquals(1, requests.get()) + response.countDown() + results.forEach { assertSame(success, it.get(TIMEOUT_SECONDS, TimeUnit.SECONDS)) } + assertEquals(1, requests.get()) + } finally { + response.countDown() + executor.shutdownNow() + } + } + + companion object { + private const val SERVER = "https://cloud.example" + private const val CERTIFICATE = "certificate" + private const val REPEATED_ENTRIES = 10 + private const val TIMEOUT_SECONDS = 5L + } +}