From 678f75fc226b80b2fd83eadbd7c7ebea68d7f910 Mon Sep 17 00:00:00 2001 From: Mike Wong Date: Tue, 19 Oct 2021 16:36:00 +0200 Subject: [PATCH 1/3] Add read-write lock to wallet data and add check before deleting transfer code on server --- .../wallet/CertificatesViewModel.kt | 15 +++++++-- .../wallet/data/WalletDataSecureStorage.kt | 33 +++++++++++++++---- .../transfercode/TransferCodeViewModel.kt | 15 +++++++-- .../transfercode/worker/TransferWorker.kt | 15 +++++++-- 4 files changed, 65 insertions(+), 13 deletions(-) diff --git a/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/CertificatesViewModel.kt b/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/CertificatesViewModel.kt index 7837d231..7e64fa6b 100644 --- a/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/CertificatesViewModel.kt +++ b/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/CertificatesViewModel.kt @@ -36,8 +36,12 @@ import ch.admin.bag.covidcertificate.wallet.transfercode.model.TransferCodeConve import ch.admin.bag.covidcertificate.wallet.transfercode.model.TransferCodeModel import ch.admin.bag.covidcertificate.wallet.transfercode.net.DeliveryRepository import ch.admin.bag.covidcertificate.wallet.transfercode.net.DeliverySpec -import kotlinx.coroutines.* +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import java.io.IOException import java.time.Instant import kotlin.collections.set @@ -257,11 +261,14 @@ class CertificatesViewModel(application: Application) : AndroidViewModel(applica val decryptedCertificates = deliveryRepository.download(transferCode.code, keyPair) if (decryptedCertificates.isNotEmpty()) { + var didReplaceTransferCode = false + decryptedCertificates.forEachIndexed { index, convertedCertificate -> val qrCodeData = convertedCertificate.qrCodeData val pdfData = convertedCertificate.pdfData + if (index == 0) { - walletDataStorage.replaceTransferCodeWithCertificate(transferCode, qrCodeData, pdfData) + didReplaceTransferCode = walletDataStorage.replaceTransferCodeWithCertificate(transferCode, qrCodeData, pdfData) val decodeState = CovidCertificateSdk.Wallet.decode(qrCodeData) conversionState = if (decodeState is DecodeState.SUCCESS) { TransferCodeConversionState.CONVERTED(decodeState.certificateHolder) @@ -272,7 +279,11 @@ class CertificatesViewModel(application: Application) : AndroidViewModel(applica } else { walletDataStorage.saveWalletDataItem(WalletDataItem.CertificateWalletData(qrCodeData, pdfData)) } + } + // Delete the transfer code on the backend and the key pair only if the certificate was stored (either by the above replace method or from another thread) + val didStoreCertificate = walletDataStorage.containsCertificate(decryptedCertificates.first().qrCodeData) + if (didReplaceTransferCode || didStoreCertificate) { try { deliveryRepository.complete(transferCode.code, keyPair) } catch (e: IOException) { diff --git a/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/data/WalletDataSecureStorage.kt b/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/data/WalletDataSecureStorage.kt index c329742a..bfdb442c 100644 --- a/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/data/WalletDataSecureStorage.kt +++ b/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/data/WalletDataSecureStorage.kt @@ -11,6 +11,7 @@ package ch.admin.bag.covidcertificate.wallet.data import android.content.Context +import androidx.core.content.edit import ch.admin.bag.covidcertificate.sdk.android.utils.EncryptedSharedPreferencesUtil import ch.admin.bag.covidcertificate.sdk.android.utils.SingletonHolder import ch.admin.bag.covidcertificate.sdk.core.models.healthcert.CertificateHolder @@ -20,6 +21,7 @@ import com.squareup.moshi.Moshi import com.squareup.moshi.Types import com.squareup.moshi.adapters.PolymorphicJsonAdapterFactory import java.time.Instant +import java.util.concurrent.locks.ReentrantLock class WalletDataSecureStorage private constructor(context: Context) { @@ -36,6 +38,7 @@ class WalletDataSecureStorage private constructor(context: Context) { } private val prefs = EncryptedSharedPreferencesUtil.initializeSharedPreferences(context, SHARED_PREFERENCES_NAME) + private val reentrantLock = ReentrantLock() fun saveWalletDataItem(dataItem: WalletDataItem) { val walletData = getWalletData().toMutableList() @@ -81,11 +84,14 @@ class WalletDataSecureStorage private constructor(context: Context) { updateWalletData(walletData) } + /** + * @return True if a transfer code was replaced with a certificate, false otherwise + */ fun replaceTransferCodeWithCertificate( transferCode: TransferCodeModel, certificateQrCodeData: String, pdfData: String? = null - ) { + ): Boolean { val walletData = getWalletData().toMutableList() val index = walletData.indexOfFirst { it is WalletDataItem.TransferCodeWalletData && it.transferCode.code == transferCode.code } @@ -95,7 +101,9 @@ class WalletDataSecureStorage private constructor(context: Context) { walletData.add(index, WalletDataItem.CertificateWalletData(certificateQrCodeData, pdfData)) } updateWalletData(walletData) + return true } + return false } fun changeWalletDataItemPosition(oldPosition: Int, newPosition: Int) { @@ -110,11 +118,17 @@ class WalletDataSecureStorage private constructor(context: Context) { } fun getWalletData(): List { - val json = prefs.getString(KEY_WALLET_DATA_ITEMS, null) - if (json == null || json.isEmpty()) { - return emptyList() + reentrantLock.lock() + + try { + val json = prefs.getString(KEY_WALLET_DATA_ITEMS, null) + if (json == null || json.isEmpty()) { + return emptyList() + } + return walletDataItemAdapter.fromJson(json) ?: emptyList() + } finally { + reentrantLock.unlock() } - return walletDataItemAdapter.fromJson(json) ?: emptyList() } fun storeCertificateLight(fullCertificate: CertificateHolder, certificateLightData: String, certificateLightQrCode: String) { @@ -170,8 +184,13 @@ class WalletDataSecureStorage private constructor(context: Context) { } private fun updateWalletData(walletData: List) { - val json = walletDataItemAdapter.toJson(walletData) - prefs.edit().putString(KEY_WALLET_DATA_ITEMS, json).apply() + reentrantLock.lock() + try { + val json = walletDataItemAdapter.toJson(walletData) + prefs.edit { putString(KEY_WALLET_DATA_ITEMS, json) } + } finally { + reentrantLock.unlock() + } } private fun List.containsCertificate(qrCodeData: String) = diff --git a/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/transfercode/TransferCodeViewModel.kt b/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/transfercode/TransferCodeViewModel.kt index 4bf2d880..98452fd2 100644 --- a/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/transfercode/TransferCodeViewModel.kt +++ b/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/transfercode/TransferCodeViewModel.kt @@ -31,7 +31,11 @@ import ch.admin.bag.covidcertificate.wallet.transfercode.model.TransferCodeConve import ch.admin.bag.covidcertificate.wallet.transfercode.model.TransferCodeModel import ch.admin.bag.covidcertificate.wallet.transfercode.net.DeliveryRepository import ch.admin.bag.covidcertificate.wallet.transfercode.net.DeliverySpec -import kotlinx.coroutines.* +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.GlobalScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch import java.io.IOException import java.security.KeyPair @@ -61,11 +65,13 @@ class TransferCodeViewModel(application: Application) : AndroidViewModel(applica val decryptedCertificates = deliveryRepository.download(transferCode.code, keyPair) if (decryptedCertificates.isNotEmpty()) { + var didReplaceTransferCode = false + decryptedCertificates.forEachIndexed { index, convertedCertificate -> val qrCodeData = convertedCertificate.qrCodeData val pdfData = convertedCertificate.pdfData if (index == 0) { - walletDataStorage.replaceTransferCodeWithCertificate(transferCode, qrCodeData, pdfData) + didReplaceTransferCode = walletDataStorage.replaceTransferCodeWithCertificate(transferCode, qrCodeData, pdfData) val decodeState = CovidCertificateSdk.Wallet.decode(qrCodeData) if (decodeState is DecodeState.SUCCESS) { conversionStateMutableLiveData.postValue(TransferCodeConversionState.CONVERTED(decodeState.certificateHolder)) @@ -76,6 +82,11 @@ class TransferCodeViewModel(application: Application) : AndroidViewModel(applica } else { walletDataStorage.saveWalletDataItem(WalletDataItem.CertificateWalletData(qrCodeData, pdfData)) } + } + + // Delete the transfer code on the backend and the key pair only if the certificate was stored (either by the above replace method or from another thread) + val didStoreCertificate = walletDataStorage.containsCertificate(decryptedCertificates.first().qrCodeData) + if (didReplaceTransferCode || didStoreCertificate) { deleteTransferCodeOnServer(transferCode, keyPair) } } else { diff --git a/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/transfercode/worker/TransferWorker.kt b/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/transfercode/worker/TransferWorker.kt index da3cf83e..4946c00c 100644 --- a/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/transfercode/worker/TransferWorker.kt +++ b/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/transfercode/worker/TransferWorker.kt @@ -11,7 +11,12 @@ package ch.admin.bag.covidcertificate.wallet.transfercode.worker import android.content.Context -import androidx.work.* +import androidx.work.BackoffPolicy +import androidx.work.CoroutineWorker +import androidx.work.ExistingPeriodicWorkPolicy +import androidx.work.PeriodicWorkRequest +import androidx.work.WorkManager +import androidx.work.WorkerParameters import ch.admin.bag.covidcertificate.common.config.ConfigModel import ch.admin.bag.covidcertificate.common.exception.TimeDeviationException import ch.admin.bag.covidcertificate.wallet.BuildConfig @@ -92,15 +97,21 @@ class TransferWorker(context: Context, workerParams: WorkerParameters) : Corouti val decryptedCertificates = deliveryRepository.download(transferCode.code, keyPair) return if (decryptedCertificates.isNotEmpty()) { + var didReplaceTransferCode = false + decryptedCertificates.forEachIndexed { index, convertedCertificate -> val qrCodeData = convertedCertificate.qrCodeData val pdfData = convertedCertificate.pdfData if (index == 0) { - walletDataStorage.replaceTransferCodeWithCertificate(transferCode, qrCodeData, pdfData) + didReplaceTransferCode = walletDataStorage.replaceTransferCodeWithCertificate(transferCode, qrCodeData, pdfData) } else { walletDataStorage.saveWalletDataItem(WalletDataItem.CertificateWalletData(qrCodeData, pdfData)) } + } + // Delete the transfer code on the backend and the key pair only if the certificate was stored (either by the above replace method or from another thread) + val didStoreCertificate = walletDataStorage.containsCertificate(decryptedCertificates.first().qrCodeData) + if (didReplaceTransferCode || didStoreCertificate) { try { deliveryRepository.complete(transferCode.code, keyPair) } catch (e: IOException) { From fb7718a6a173190f8fbb756d20deeba002695dd9 Mon Sep 17 00:00:00 2001 From: Mike Wong Date: Wed, 20 Oct 2021 17:15:41 +0200 Subject: [PATCH 2/3] Migrate transfer code validity --- .../covidcertificate/wallet/MainApplication.kt | 14 ++++++++++++++ .../wallet/data/WalletDataSecureStorage.kt | 2 +- .../wallet/data/WalletSecureStorage.kt | 7 +++++++ .../TransferCodeCreationViewModel.kt | 5 ++++- .../transfercode/model/TransferCodeModel.kt | 16 ++++++++++------ 5 files changed, 36 insertions(+), 8 deletions(-) diff --git a/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/MainApplication.kt b/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/MainApplication.kt index 2befa860..eaa53564 100644 --- a/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/MainApplication.kt +++ b/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/MainApplication.kt @@ -35,6 +35,7 @@ class MainApplication : Application() { CovidCertificateSdk.init(this, EnvironmentUtil.getSdkEnvironment()) migrateCertificatesToWalletData() + migrateTransferCodeValidity() setupTransferWorker() } @@ -56,6 +57,19 @@ class MainApplication : Application() { } } + private fun migrateTransferCodeValidity() { + val walletStorage = WalletSecureStorage.getInstance(this) + if (!walletStorage.getMigratedTransferCodeValidity()) { + // Reading the wallet data once from the storage and writing it again immediately is enough to migrate the validity. + // The data class constructor defines the new fields with a default value, so it is automatically set when deserializing + val walletDataStorage = WalletDataSecureStorage.getInstance(this) + val walletDataItems = walletDataStorage.getWalletData() + walletDataStorage.updateWalletData(walletDataItems) + + walletStorage.setMigratedTransferCodeValidity(true) + } + } + private fun setupTransferWorker() { ProcessLifecycleOwner.get().lifecycle.addObserver(object : LifecycleObserver { @OnLifecycleEvent(Lifecycle.Event.ON_START) diff --git a/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/data/WalletDataSecureStorage.kt b/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/data/WalletDataSecureStorage.kt index bfdb442c..14db4504 100644 --- a/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/data/WalletDataSecureStorage.kt +++ b/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/data/WalletDataSecureStorage.kt @@ -183,7 +183,7 @@ class WalletDataSecureStorage private constructor(context: Context) { } } - private fun updateWalletData(walletData: List) { + fun updateWalletData(walletData: List) { reentrantLock.lock() try { val json = walletDataItemAdapter.toJson(walletData) diff --git a/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/data/WalletSecureStorage.kt b/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/data/WalletSecureStorage.kt index 4c483987..c89e66ea 100644 --- a/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/data/WalletSecureStorage.kt +++ b/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/data/WalletSecureStorage.kt @@ -21,6 +21,7 @@ class WalletSecureStorage private constructor(context: Context) { private const val PREFERENCES = "SecureStorage" private const val KEY_ONBOARDING_COMPLETED = "onboarding_completed" private const val KEY_MIGRATED_CERTIFICATES_TO_WALLET_DATA = "KEY_MIGRATED_CERTIFICATES_TO_WALLET_DATA" + private const val KEY_MIGRATED_TRANSFER_CODE_VALIDITY = "KEY_MIGRATED_TRANSFER_CODE_VALIDITY" private const val KEY_CERTIFICATE_LIGHT_UPDATEBOARDING_COMPLETED = "KEY_CERTIFICATE_LIGHT_UPDATEBOARDING_COMPLETED" private const val KEY_TRANSFER_CODE_PUBLIC_KEY_PREFIX = "TRANSFER_CODE_PUBLIC_KEY_" private const val KEY_TRANSFER_CODE_PRIVATE_KEY_PREFIX = "TRANSFER_CODE_PRIVATE_KEY_" @@ -41,6 +42,12 @@ class WalletSecureStorage private constructor(context: Context) { putBoolean(KEY_MIGRATED_CERTIFICATES_TO_WALLET_DATA, migrated) } + fun getMigratedTransferCodeValidity() = prefs.getBoolean(KEY_MIGRATED_TRANSFER_CODE_VALIDITY, false) + + fun setMigratedTransferCodeValidity(migrated: Boolean) = prefs.edit { + putBoolean(KEY_MIGRATED_TRANSFER_CODE_VALIDITY, migrated) + } + fun getCertificateLightUpdateboardingCompleted() = prefs.getBoolean(KEY_CERTIFICATE_LIGHT_UPDATEBOARDING_COMPLETED, false) fun setCertificateLightUpdateboardingCompleted(completed: Boolean) = prefs.edit { diff --git a/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/transfercode/TransferCodeCreationViewModel.kt b/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/transfercode/TransferCodeCreationViewModel.kt index 79b1b748..8889180f 100644 --- a/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/transfercode/TransferCodeCreationViewModel.kt +++ b/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/transfercode/TransferCodeCreationViewModel.kt @@ -37,6 +37,7 @@ import kotlinx.coroutines.withContext import java.io.IOException import java.security.KeyPair import java.time.Instant +import java.time.temporal.ChronoUnit class TransferCodeCreationViewModel(application: Application) : AndroidViewModel(application) { @@ -73,7 +74,9 @@ class TransferCodeCreationViewModel(application: Application) : AndroidViewModel when (registrationResponse) { TransferCodeCreationResponse.SUCCESSFUL -> { val now = Instant.now() - val transferCodeModel = TransferCodeModel(transferCode, now, now) + val expiresAt = now.plus(30, ChronoUnit.DAYS) + val failsAt = now.plus(72, ChronoUnit.HOURS) + val transferCodeModel = TransferCodeModel(transferCode, now, now, expiresAt, failsAt) walletDataStorage.saveWalletDataItem(WalletDataItem.TransferCodeWalletData(transferCodeModel)) creationStateMutableLiveData.postValue(TransferCodeCreationState.SUCCESS(transferCodeModel)) } diff --git a/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/transfercode/model/TransferCodeModel.kt b/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/transfercode/model/TransferCodeModel.kt index 1524c621..d8bbcdf8 100644 --- a/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/transfercode/model/TransferCodeModel.kt +++ b/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/transfercode/model/TransferCodeModel.kt @@ -17,23 +17,27 @@ import java.time.temporal.ChronoUnit import java.util.concurrent.TimeUnit import kotlin.math.roundToInt +/** + * The expiresAt and failsAt default values are set to 30d/72h due to the extended validity starting with the 2.7.0 release. + * This default value should only be applied during migration, in all other cases, the correct validity range should be set based on + * the server configuration. + */ @JsonClass(generateAdapter = true) data class TransferCodeModel( val code: String, val creationTimestamp: Instant, val lastUpdatedTimestamp: Instant, + val expiresAtTimestamp: Instant = creationTimestamp.plus(30, ChronoUnit.DAYS), + val failsAtTimestamp: Instant = expiresAtTimestamp.plus(72, ChronoUnit.HOURS), ): Serializable { - val expirationTimestamp = creationTimestamp.plus(30, ChronoUnit.DAYS) - val failureTimestamp = expirationTimestamp.plus(72, ChronoUnit.HOURS) - - fun isExpired() = expirationTimestamp.isBefore(Instant.now()) + fun isExpired() = expiresAtTimestamp.isBefore(Instant.now()) - fun isFailed() = failureTimestamp.isBefore(Instant.now()) + fun isFailed() = failsAtTimestamp.isBefore(Instant.now()) fun getDaysUntilExpiration(): Int { val now = Instant.now().toEpochMilli() - val diff = expirationTimestamp.toEpochMilli() - now + val diff = expiresAtTimestamp.toEpochMilli() - now return (diff.toDouble() / TimeUnit.DAYS.toMillis(1)).roundToInt() } } \ No newline at end of file From 0bf10a9da2880f85b748fa4c892009cde47cfbca Mon Sep 17 00:00:00 2001 From: Mike Wong Date: Thu, 21 Oct 2021 10:16:08 +0200 Subject: [PATCH 3/3] Fix wrong migration value --- .../wallet/transfercode/model/TransferCodeModel.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/transfercode/model/TransferCodeModel.kt b/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/transfercode/model/TransferCodeModel.kt index d8bbcdf8..991735d8 100644 --- a/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/transfercode/model/TransferCodeModel.kt +++ b/wallet/src/main/java/ch/admin/bag/covidcertificate/wallet/transfercode/model/TransferCodeModel.kt @@ -18,16 +18,16 @@ import java.util.concurrent.TimeUnit import kotlin.math.roundToInt /** - * The expiresAt and failsAt default values are set to 30d/72h due to the extended validity starting with the 2.7.0 release. - * This default value should only be applied during migration, in all other cases, the correct validity range should be set based on - * the server configuration. + * The expiresAt and failsAt default values are set to 7d/72h due to the extended validity starting with the 2.7.0 release. + * Existing transfer codes should automatically be migrated with these default values, while newer codes should set the correct + * validity range based on the server configuration. */ @JsonClass(generateAdapter = true) data class TransferCodeModel( val code: String, val creationTimestamp: Instant, val lastUpdatedTimestamp: Instant, - val expiresAtTimestamp: Instant = creationTimestamp.plus(30, ChronoUnit.DAYS), + val expiresAtTimestamp: Instant = creationTimestamp.plus(7, ChronoUnit.DAYS), val failsAtTimestamp: Instant = expiresAtTimestamp.plus(72, ChronoUnit.HOURS), ): Serializable {