Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

Fix automatic data reset on for users affected by encryption issues (EXPOSUREAPP-3313, EXPOSUREAPP-3335) #1433

Merged
merged 3 commits into from
Oct 21, 2020
Merged

Fix automatic data reset on for users affected by encryption issues (EXPOSUREAPP-3313, EXPOSUREAPP-3335) #1433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8baf414
I overlooked that `KeyException` is a subclass of `GeneralSecurityExc…
d4rken Oct 19, 2020
557adc4
Add additional specific test cases for nested exceptions.
d4rken Oct 20, 2020
e4e2280
The dialog shouldn't automatically dismiss when the user clicks details.
d4rken Oct 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/main/MainFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class MainFragment : Fragment(R.layout.fragment_main) {
if (errorResetTool.isResetNoticeToBeShown) {
RecoveryByResetDialogFactory(this).showDialog(
detailsLink = R.string.errors_generic_text_catastrophic_error_encryption_failure,
onDismiss = {
onPositive = {
errorResetTool.isResetNoticeToBeShown = false
}
)
Expand Down
17 changes: 10 additions & 7 deletions ...a-Warn-App/src/main/java/de/rki/coronawarnapp/util/errors/RecoveryByResetDialogFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,20 @@ class RecoveryByResetDialogFactory(private val fragment: Fragment) {

fun showDialog(
@StringRes detailsLink: Int,
onDismiss: () -> Unit
onPositive: () -> Unit
) {
AlertDialog.Builder(context)
val dialog = AlertDialog.Builder(context)
.setTitle(R.string.errors_generic_headline)
.setMessage(R.string.errors_generic_text_catastrophic_error_recovery_via_reset)
.setCancelable(false)
.setOnDismissListener { onDismiss() }
.setNeutralButton(R.string.errors_generic_button_negative) { _, _ ->
ExternalActionHelper.openUrl(fragment, context.getString(detailsLink))
.setNeutralButton(R.string.errors_generic_button_negative, null)
.setPositiveButton(R.string.errors_generic_button_positive) { _, _ ->
onPositive()
}
.setPositiveButton(R.string.errors_generic_button_positive) { _, _ -> }
.show()
.create()
dialog.show()
dialog.getButton(AlertDialog.BUTTON_NEUTRAL)?.setOnClickListener {
ExternalActionHelper.openUrl(fragment, context.getString(detailsLink))
}
}
}
4 changes: 2 additions & 2 deletions Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/security/EncryptionErrorResetTool.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ class EncryptionErrorResetTool @Inject constructor(
}
isResetWindowConsumed = true

val keyException = error.causes().singleOrNull { it is GeneralSecurityException }
if (keyException == null) {
val keyException = error.causes().lastOrNull()
if (keyException == null || keyException !is GeneralSecurityException) {
Timber.v("Error has no GeneralSecurityException as cause -> no reset.")
return false
}
Expand Down
82 changes: 82 additions & 0 deletions Corona-Warn-App/src/test/java/de/rki/coronawarnapp/util/security/EncryptionResetToolTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ import org.junit.jupiter.api.Test
import testhelpers.BaseIOTest
import testhelpers.preferences.MockSharedPreferences
import java.io.File
import java.io.IOException
import java.security.GeneralSecurityException
import java.security.KeyException
import java.security.KeyStoreException

class EncryptionResetToolTest : BaseIOTest() {
Expand Down Expand Up @@ -169,6 +171,86 @@ class EncryptionResetToolTest : BaseIOTest() {
}
}

@Test
fun `nested exception may have the same base exception type, ie GeneralSecurityException`() {
// https://github.com/corona-warn-app/cwa-app-android/issues/642#issuecomment-712188157
createMockFiles()

createInstance().tryResetIfNecessary(
KeyException( // subclass of GeneralSecurityException
"Permantly failed to instantiate encrypted preferences",
SecurityException(
"Could not decrypt key. decryption failed",
GeneralSecurityException("decryption failed")
)
)
) shouldBe true

encryptedPrefsFile.exists() shouldBe false
encryptedDatabaseFile.exists() shouldBe false

mockPreferences.dataMapPeek.apply {
this["ea1851.reset.performedAt"] shouldNotBe null
this["ea1851.reset.windowconsumed"] shouldBe true
this["ea1851.reset.shownotice"] shouldBe true
}
}

@Test
fun `exception check does not care about the first exception type`() {
createMockFiles()

createInstance().tryResetIfNecessary(
CwaSecurityException(
KeyException( // subclass of GeneralSecurityException
"Permantly failed to instantiate encrypted preferences",
SecurityException(
"Could not decrypt key. decryption failed",
GeneralSecurityException("decryption failed")
)
)
)
) shouldBe true

encryptedPrefsFile.exists() shouldBe false
encryptedDatabaseFile.exists() shouldBe false

mockPreferences.dataMapPeek.apply {
this["ea1851.reset.performedAt"] shouldNotBe null
this["ea1851.reset.windowconsumed"] shouldBe true
this["ea1851.reset.shownotice"] shouldBe true
}
}

@Test
fun `exception check DOES care about the most nested exception`() {
createMockFiles()

createInstance().tryResetIfNecessary(
CwaSecurityException(
KeyException( // subclass of GeneralSecurityException
"Permantly failed to instantiate encrypted preferences",
SecurityException(
"Could not decrypt key. decryption failed",
GeneralSecurityException(
"decryption failed",
IOException("I am unexpeted")
)
)
)
)
) shouldBe false

encryptedPrefsFile.exists() shouldBe true
encryptedDatabaseFile.exists() shouldBe true

mockPreferences.dataMapPeek.apply {
this["ea1851.reset.performedAt"] shouldBe null
this["ea1851.reset.windowconsumed"] shouldBe true
this["ea1851.reset.shownotice"] shouldBe null
}
}

@Test
fun `we want only a specific type of GeneralSecurityException`() {
createMockFiles()
Expand Down