From b29bf3825975f9c536616904c353870dc435f66f Mon Sep 17 00:00:00 2001
From: Matthias Urhahn <matthias.urhahn@sap.com>
Date: Wed, 28 Oct 2020 15:17:40 +0100
Subject: [PATCH] Retry the reset for v1.6.0+ as the failed reset attempt in
 1.5.0 may have consumed the reset attempt already. (#1492)

---
 .../util/security/EncryptionErrorResetTool.kt | 15 ++++--
 .../util/security/EncryptionResetToolTest.kt  | 54 ++++++++++++++-----
 2 files changed, 52 insertions(+), 17 deletions(-)

diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/security/EncryptionErrorResetTool.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/security/EncryptionErrorResetTool.kt
index 329cff723..297085ac4 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/security/EncryptionErrorResetTool.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/security/EncryptionErrorResetTool.kt
@@ -4,6 +4,7 @@ import android.content.Context
 import android.content.SharedPreferences
 import androidx.core.content.edit
 import de.rki.coronawarnapp.storage.DATABASE_NAME
+import de.rki.coronawarnapp.util.TimeStamper
 import de.rki.coronawarnapp.util.di.AppContext
 import de.rki.coronawarnapp.util.errors.causes
 import org.joda.time.Instant
@@ -23,7 +24,8 @@ import javax.inject.Singleton
  */
 @Singleton
 class EncryptionErrorResetTool @Inject constructor(
-    @AppContext private val context: Context
+    @AppContext private val context: Context,
+    private val timeStamper: TimeStamper
 ) {
     // https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/java/android/app/ContextImpl.java;drc=3b8e8d76315f6718a982d5e6a019b3aa4f634bcd;l=626
     private val encryptedPreferencesFile by lazy {
@@ -63,9 +65,9 @@ class EncryptionErrorResetTool @Inject constructor(
         }
 
     fun tryResetIfNecessary(error: Throwable): Boolean {
-        Timber.w(error, "isRecoveryWarranted()")
+        Timber.w(error, "tryResetIfNecessary()")
 
-        // We only reset for the first error encountered after upgrading to 1.4.0+
+        // We only try to reset once, if the error reoccurs, on-going resets is not the solution.
         if (isResetWindowConsumed) {
             Timber.v("Reset window has been consumed -> no reset.")
             return false
@@ -122,7 +124,7 @@ class EncryptionErrorResetTool @Inject constructor(
             return false
         }
 
-        resetPerformedAt = Instant.now()
+        resetPerformedAt = timeStamper.nowUTC
         isResetNoticeToBeShown = true
 
         return true
@@ -130,7 +132,10 @@ class EncryptionErrorResetTool @Inject constructor(
 
     companion object {
         private const val PKEY_EA1851_RESET_PERFORMED_AT = "ea1851.reset.performedAt"
-        private const val PKEY_EA1851_WAS_WINDOW_CONSUMED = "ea1851.reset.windowconsumed"
+
+        @Suppress("unused")
+        private const val PKEY_EA1851_WAS_WINDOW_CONSUMED_OLD = "ea1851.reset.windowconsumed"
+        private const val PKEY_EA1851_WAS_WINDOW_CONSUMED = "ea1851.reset.windowconsumed.160"
         private const val PKEY_EA1851_SHOW_RESET_NOTICE = "ea1851.reset.shownotice"
     }
 }
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/util/security/EncryptionResetToolTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/util/security/EncryptionResetToolTest.kt
index 7daab4ac9..f1612cffd 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/util/security/EncryptionResetToolTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/util/security/EncryptionResetToolTest.kt
@@ -1,13 +1,15 @@
 package de.rki.coronawarnapp.util.security
 
 import android.content.Context
+import androidx.core.content.edit
 import de.rki.coronawarnapp.exception.CwaSecurityException
+import de.rki.coronawarnapp.util.TimeStamper
 import io.kotest.matchers.shouldBe
-import io.kotest.matchers.shouldNotBe
 import io.mockk.MockKAnnotations
 import io.mockk.clearAllMocks
 import io.mockk.every
 import io.mockk.impl.annotations.MockK
+import org.joda.time.Instant
 import org.junit.jupiter.api.AfterEach
 import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
@@ -19,8 +21,8 @@ import java.security.KeyStoreException
 
 class EncryptionResetToolTest : BaseIOTest() {
 
-    @MockK
-    lateinit var context: Context
+    @MockK lateinit var context: Context
+    @MockK lateinit var timeStamper: TimeStamper
     private lateinit var mockPreferences: MockSharedPreferences
 
     private val testDir = File(IO_TEST_BASEDIR, this::class.simpleName!!)
@@ -42,6 +44,8 @@ class EncryptionResetToolTest : BaseIOTest() {
                 Context.MODE_PRIVATE
             )
         } returns mockPreferences
+
+        every { timeStamper.nowUTC } returns Instant.ofEpochMilli(1234567890L)
     }
 
     @AfterEach
@@ -52,7 +56,8 @@ class EncryptionResetToolTest : BaseIOTest() {
     }
 
     private fun createInstance() = EncryptionErrorResetTool(
-        context = context
+        context = context,
+        timeStamper = timeStamper
     )
 
     private fun createMockFiles() {
@@ -144,8 +149,33 @@ class EncryptionResetToolTest : BaseIOTest() {
         encryptedDatabaseFile.exists() shouldBe false
 
         mockPreferences.dataMapPeek.apply {
-            this["ea1851.reset.performedAt"] shouldNotBe null
+            this["ea1851.reset.performedAt"] shouldBe 1234567890L
+            this["ea1851.reset.windowconsumed.160"] shouldBe true
+            this["ea1851.reset.shownotice"] shouldBe true
+        }
+    }
+
+    @Test
+    fun `the previous reset attempt from 1_5_0 is ignored`() {
+        mockPreferences.edit { putBoolean("ea1851.reset.windowconsumed", true) }
+
+        mockPreferences.dataMapPeek.apply {
+            this["ea1851.reset.performedAt"] shouldBe null
+            this["ea1851.reset.windowconsumed"] shouldBe true
+            this["ea1851.reset.windowconsumed.160"] shouldBe null
+            this["ea1851.reset.shownotice"] shouldBe null
+        }
+
+        createMockFiles()
+
+        createInstance().tryResetIfNecessary(
+            GeneralSecurityException("decryption failed")
+        ) shouldBe true
+
+        mockPreferences.dataMapPeek.apply {
+            this["ea1851.reset.performedAt"] shouldBe 1234567890L
             this["ea1851.reset.windowconsumed"] shouldBe true
+            this["ea1851.reset.windowconsumed.160"] shouldBe true
             this["ea1851.reset.shownotice"] shouldBe true
         }
     }
@@ -163,8 +193,8 @@ class EncryptionResetToolTest : BaseIOTest() {
         encryptedDatabaseFile.exists() shouldBe false
 
         mockPreferences.dataMapPeek.apply {
-            this["ea1851.reset.performedAt"] shouldNotBe null
-            this["ea1851.reset.windowconsumed"] shouldBe true
+            this["ea1851.reset.performedAt"] shouldBe 1234567890L
+            this["ea1851.reset.windowconsumed.160"] shouldBe true
             this["ea1851.reset.shownotice"] shouldBe true
         }
     }
@@ -182,7 +212,7 @@ class EncryptionResetToolTest : BaseIOTest() {
 
         mockPreferences.dataMapPeek.apply {
             this["ea1851.reset.performedAt"] shouldBe null
-            this["ea1851.reset.windowconsumed"] shouldBe true
+            this["ea1851.reset.windowconsumed.160"] shouldBe true
             this["ea1851.reset.shownotice"] shouldBe null
         }
     }
@@ -202,7 +232,7 @@ class EncryptionResetToolTest : BaseIOTest() {
 
         mockPreferences.dataMapPeek.apply {
             this["ea1851.reset.performedAt"] shouldBe null
-            this["ea1851.reset.windowconsumed"] shouldBe true
+            this["ea1851.reset.windowconsumed.160"] shouldBe true
             this["ea1851.reset.shownotice"] shouldBe null
         }
     }
@@ -220,7 +250,7 @@ class EncryptionResetToolTest : BaseIOTest() {
 
         mockPreferences.dataMapPeek.apply {
             this["ea1851.reset.performedAt"] shouldBe null
-            this["ea1851.reset.windowconsumed"] shouldBe true
+            this["ea1851.reset.windowconsumed.160"] shouldBe true
             this["ea1851.reset.shownotice"] shouldBe null
         }
     }
@@ -241,7 +271,7 @@ class EncryptionResetToolTest : BaseIOTest() {
 
         mockPreferences.dataMapPeek.apply {
             this["ea1851.reset.performedAt"] shouldBe null
-            this["ea1851.reset.windowconsumed"] shouldBe true
+            this["ea1851.reset.windowconsumed.160"] shouldBe true
             this["ea1851.reset.shownotice"] shouldBe null
         }
     }
@@ -262,7 +292,7 @@ class EncryptionResetToolTest : BaseIOTest() {
 
         mockPreferences.dataMapPeek.apply {
             this["ea1851.reset.performedAt"] shouldBe null
-            this["ea1851.reset.windowconsumed"] shouldBe true
+            this["ea1851.reset.windowconsumed.160"] shouldBe true
             this["ea1851.reset.shownotice"] shouldBe null
         }
     }
-- 
GitLab