From c3e990b31e78cb2e6424d5908d8a75a15d5e9e9b Mon Sep 17 00:00:00 2001
From: Matthias Urhahn <matthias.urhahn@sap.com>
Date: Fri, 5 Mar 2021 11:33:59 +0100
Subject: [PATCH] Refactored VerificationKeys.kt: Made the class more general
purpose (DEV) #2530
---
.../coronawarnapp/ui/statistics/Statistics.kt | 6 +-
.../util/security/VerificationKeysTest.kt | 22 +++---
.../sources/remote/AppConfigServer.kt | 10 ++-
.../statistics/source/StatisticsServer.kt | 11 ++-
.../util/security/SignatureValidation.kt | 75 +++++++++++++++++++
.../util/security/VerificationKeys.kt | 75 -------------------
.../sources/remote/AppConfigServerTest.kt | 12 +--
.../statistics/source/StatisticsServerTest.kt | 12 +--
8 files changed, 116 insertions(+), 107 deletions(-)
create mode 100644 Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/security/SignatureValidation.kt
delete mode 100644 Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/security/VerificationKeys.kt
diff --git a/Corona-Warn-App/src/androidTest/java/de/rki/coronawarnapp/ui/statistics/Statistics.kt b/Corona-Warn-App/src/androidTest/java/de/rki/coronawarnapp/ui/statistics/Statistics.kt
index f4a6f85e1..1e15b9f80 100644
--- a/Corona-Warn-App/src/androidTest/java/de/rki/coronawarnapp/ui/statistics/Statistics.kt
+++ b/Corona-Warn-App/src/androidTest/java/de/rki/coronawarnapp/ui/statistics/Statistics.kt
@@ -9,7 +9,7 @@ import de.rki.coronawarnapp.statistics.StatisticsData
import de.rki.coronawarnapp.statistics.StatisticsModule
import de.rki.coronawarnapp.statistics.source.StatisticsParser
import de.rki.coronawarnapp.statistics.source.StatisticsServer
-import de.rki.coronawarnapp.util.security.VerificationKeys
+import de.rki.coronawarnapp.util.security.SignatureValidation
import de.rki.coronawarnapp.util.serialization.SerializationModule
import io.mockk.every
import io.mockk.mockk
@@ -34,7 +34,7 @@ object Statistics {
val httpClient = HttpModule().defaultHttpClient()
val cdnClient = cdnModule.cdnHttpClient(httpClient)
val url = cdnModule.provideDownloadServerUrl(environmentSetup)
- val verificationKeys = VerificationKeys(environmentSetup)
+ val signatureValidation = SignatureValidation(environmentSetup)
val gsonFactory = GsonConverterFactory.create()
val statisticsServer = StatisticsServer(
@@ -47,7 +47,7 @@ object Statistics {
)
},
cache = cache,
- verificationKeys = verificationKeys
+ signatureValidation = signatureValidation
)
return runBlocking {
diff --git a/Corona-Warn-App/src/androidTest/java/de/rki/coronawarnapp/util/security/VerificationKeysTest.kt b/Corona-Warn-App/src/androidTest/java/de/rki/coronawarnapp/util/security/VerificationKeysTest.kt
index 817159d68..8e8f54718 100644
--- a/Corona-Warn-App/src/androidTest/java/de/rki/coronawarnapp/util/security/VerificationKeysTest.kt
+++ b/Corona-Warn-App/src/androidTest/java/de/rki/coronawarnapp/util/security/VerificationKeysTest.kt
@@ -23,33 +23,33 @@ class VerificationKeysTest {
every { environmentSetup.appConfigVerificationKey } returns PUB_KEY
}
- private fun createTool() = VerificationKeys(environmentSetup)
+ private fun createTool() = SignatureValidation(environmentSetup)
@Test
fun goodBinaryAndSignature() {
val tool = createTool()
- tool.hasInvalidSignature(
+ tool.hasValidSignature(
GOOD_BINARY.decodeHex().toByteArray(),
- GOOD_SIGNATURE.decodeHex().toByteArray()
- ) shouldBe false
+ SignatureValidation.parseTEKStyleSignature(GOOD_SIGNATURE.decodeHex().toByteArray())
+ ) shouldBe true
}
@Test
fun badBinaryGoodSignature() {
val tool = createTool()
- tool.hasInvalidSignature(
+ tool.hasValidSignature(
"123ABC".decodeHex().toByteArray(),
- GOOD_SIGNATURE.decodeHex().toByteArray()
- ) shouldBe true
+ SignatureValidation.parseTEKStyleSignature(GOOD_SIGNATURE.decodeHex().toByteArray())
+ ) shouldBe false
}
@Test
fun goodBinaryBadSignature() {
val tool = createTool()
shouldThrow<CwaSecurityException> {
- tool.hasInvalidSignature(
+ tool.hasValidSignature(
GOOD_BINARY.decodeHex().toByteArray(),
- "123ABC".decodeHex().toByteArray()
+ SignatureValidation.parseTEKStyleSignature("123ABC".decodeHex().toByteArray())
)
}
}
@@ -58,9 +58,9 @@ class VerificationKeysTest {
fun badEverything() {
val tool = createTool()
shouldThrow<CwaSecurityException> {
- tool.hasInvalidSignature(
+ tool.hasValidSignature(
"123ABC".decodeHex().toByteArray(),
- "123ABC".decodeHex().toByteArray()
+ SignatureValidation.parseTEKStyleSignature("123ABC".decodeHex().toByteArray())
)
}
}
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/sources/remote/AppConfigServer.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/sources/remote/AppConfigServer.kt
index 39b3c2cb6..02bbd1094 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/sources/remote/AppConfigServer.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/sources/remote/AppConfigServer.kt
@@ -12,7 +12,7 @@ import de.rki.coronawarnapp.util.TimeStamper
import de.rki.coronawarnapp.util.ZipHelper.readIntoMap
import de.rki.coronawarnapp.util.ZipHelper.unzip
import de.rki.coronawarnapp.util.retrofit.etag
-import de.rki.coronawarnapp.util.security.VerificationKeys
+import de.rki.coronawarnapp.util.security.SignatureValidation
import okhttp3.CacheControl
import org.joda.time.Duration
import org.joda.time.Instant
@@ -26,7 +26,7 @@ import javax.inject.Inject
@Reusable
class AppConfigServer @Inject constructor(
private val api: Lazy<AppConfigApiV2>,
- private val verificationKeys: VerificationKeys,
+ private val signatureValidation: SignatureValidation,
private val timeStamper: TimeStamper,
private val testSettings: TestSettings
) {
@@ -49,7 +49,11 @@ class AppConfigServer @Inject constructor(
throw ApplicationConfigurationInvalidException(message = "Unknown files: ${fileMap.keys}")
}
- if (verificationKeys.hasInvalidSignature(exportBinary, exportSignature)) {
+ val hasValidSignature = signatureValidation.hasValidSignature(
+ exportBinary,
+ SignatureValidation.parseTEKStyleSignature(exportSignature)
+ )
+ if (!hasValidSignature) {
throw ApplicationConfigurationCorruptException()
}
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/statistics/source/StatisticsServer.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/statistics/source/StatisticsServer.kt
index 11526a01c..10eea6f67 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/statistics/source/StatisticsServer.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/statistics/source/StatisticsServer.kt
@@ -5,7 +5,7 @@ import dagger.Reusable
import de.rki.coronawarnapp.statistics.Statistics
import de.rki.coronawarnapp.util.ZipHelper.readIntoMap
import de.rki.coronawarnapp.util.ZipHelper.unzip
-import de.rki.coronawarnapp.util.security.VerificationKeys
+import de.rki.coronawarnapp.util.security.SignatureValidation
import okhttp3.Cache
import retrofit2.HttpException
import timber.log.Timber
@@ -15,7 +15,7 @@ import javax.inject.Inject
@Reusable
class StatisticsServer @Inject constructor(
private val api: Lazy<StatisticsApiV1>,
- private val verificationKeys: VerificationKeys,
+ private val signatureValidation: SignatureValidation,
@Statistics val cache: Cache
) {
@@ -37,7 +37,12 @@ class StatisticsServer @Inject constructor(
throw IOException("Unknown files: ${fileMap.keys}")
}
- if (verificationKeys.hasInvalidSignature(exportBinary, exportSignature)) {
+ val hasValidSignature = signatureValidation.hasValidSignature(
+ exportBinary,
+ SignatureValidation.parseTEKStyleSignature(exportSignature)
+ )
+
+ if (!hasValidSignature) {
throw InvalidStatisticsSignatureException(message = "Statistics signature did not match.")
}
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/security/SignatureValidation.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/security/SignatureValidation.kt
new file mode 100644
index 000000000..a3221d6d2
--- /dev/null
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/security/SignatureValidation.kt
@@ -0,0 +1,75 @@
+package de.rki.coronawarnapp.util.security
+
+import android.security.keystore.KeyProperties
+import android.util.Base64
+import de.rki.coronawarnapp.environment.EnvironmentSetup
+import de.rki.coronawarnapp.exception.CwaSecurityException
+import de.rki.coronawarnapp.server.protocols.external.exposurenotification.TemporaryExposureKeySignatureList.TEKSignatureList
+import timber.log.Timber
+import java.security.KeyFactory
+import java.security.Signature
+import java.security.spec.X509EncodedKeySpec
+import javax.inject.Inject
+import javax.inject.Singleton
+
+@Singleton
+class SignatureValidation @Inject constructor(
+ private val environmentSetup: EnvironmentSetup
+) {
+
+ private val keyFactory by lazy {
+ KeyFactory.getInstance(KeyProperties.KEY_ALGORITHM_EC)
+ }
+ private val signature by lazy {
+ Signature.getInstance(SecurityConstants.EXPORT_FILE_SIGNATURE_VERIFICATION_ALGORITHM)
+ }
+
+ // Public keys within this server environment
+ private val publicKeys by lazy {
+ environmentSetup.appConfigVerificationKey.split(KEY_DELIMITER)
+ .mapNotNull { pubKeyBase64 -> Base64.decode(pubKeyBase64, Base64.DEFAULT) }
+ .map { pubKeyBinary -> keyFactory.generatePublic(X509EncodedKeySpec(pubKeyBinary)) }
+ .onEach { Timber.tag(TAG).v("ENV PubKey: %s", it) }
+ }
+
+ fun hasValidSignature(toVerify: ByteArray, signatureList: Sequence<ByteArray>): Boolean = try {
+ val validSignatures = signature.findMatchingPublicKeys(toVerify, signatureList)
+ Timber.tag(TAG).v("${validSignatures.size} valid signatures found")
+
+ validSignatures.isNotEmpty().also {
+ if (it) Timber.tag(TAG).d("Valid signatures found.")
+ else Timber.tag(TAG).w("No valid signature found.")
+ }
+ } catch (e: Exception) {
+ throw CwaSecurityException(e)
+ }
+
+ private fun Signature.findMatchingPublicKeys(
+ toVerify: ByteArray,
+ signatures: Sequence<ByteArray>
+ ) = publicKeys.filter { publicKey ->
+ for (signature in signatures) {
+ initVerify(publicKey)
+ update(toVerify)
+ if (verify(signature)) return@filter true
+ }
+ return@filter false
+ }
+
+ companion object {
+ private const val KEY_DELIMITER = ","
+ private val TAG = SignatureValidation::class.java.simpleName
+
+ fun parseTEKStyleSignature(signatureListProto: ByteArray) = try {
+ TEKSignatureList
+ .parseFrom(signatureListProto)
+ .signaturesList
+ .asSequence()
+ .onEach { Timber.tag(TAG).v(it.toString()) }
+ .mapNotNull { it.signature.toByteArray() }
+ } catch (e: Exception) {
+ Timber.w("%s is not a valid TEKSignatureList", signatureListProto)
+ throw CwaSecurityException(e)
+ }
+ }
+}
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/security/VerificationKeys.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/security/VerificationKeys.kt
deleted file mode 100644
index 1ba8319fe..000000000
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/security/VerificationKeys.kt
+++ /dev/null
@@ -1,75 +0,0 @@
-package de.rki.coronawarnapp.util.security
-
-import android.security.keystore.KeyProperties
-import android.util.Base64
-import de.rki.coronawarnapp.environment.EnvironmentSetup
-import de.rki.coronawarnapp.server.protocols.external.exposurenotification.TemporaryExposureKeySignatureList.TEKSignatureList
-import timber.log.Timber
-import java.security.KeyFactory
-import java.security.Signature
-import java.security.spec.X509EncodedKeySpec
-import javax.inject.Inject
-import javax.inject.Singleton
-
-@Singleton
-class VerificationKeys @Inject constructor(
- private val environmentSetup: EnvironmentSetup
-) {
- companion object {
- private const val KEY_DELIMITER = ","
- private val TAG = VerificationKeys::class.java.simpleName
- }
-
- private val keyFactory = KeyFactory.getInstance(KeyProperties.KEY_ALGORITHM_EC)
- private val signature =
- Signature.getInstance(SecurityConstants.EXPORT_FILE_SIGNATURE_VERIFICATION_ALGORITHM)
-
- fun hasInvalidSignature(
- export: ByteArray,
- signatureListBinary: ByteArray
- ): Boolean = SecurityHelper.withSecurityCatch {
- signature.getValidSignaturesForExport(export, signatureListBinary)
- .isEmpty()
- .also {
- if (it) Timber.tag(TAG).d("export is invalid")
- else Timber.tag(TAG).d("export is valid")
- }
- }
-
- private fun Signature.getValidSignaturesForExport(
- export: ByteArray?,
- signatures: ByteArray?
- ) = getKeysForSignatureVerificationFilteredByEnvironment()
- .filter { publicKey ->
- var verified = false
- getTEKSignaturesForEnvironment(signatures).forEach { tek ->
- initVerify(publicKey)
- update(export)
- if (verify(tek)) verified = true
- }
- verified
- }
- .also { Timber.tag(TAG).v("${it.size} valid signatures found") }
-
- private fun getKeysForSignatureVerificationFilteredByEnvironment() =
- environmentSetup.appConfigVerificationKey.split(KEY_DELIMITER)
- .mapNotNull { delimitedString ->
- Base64.decode(delimitedString, Base64.DEFAULT)
- }.map { binaryPublicKey ->
- keyFactory.generatePublic(
- X509EncodedKeySpec(
- binaryPublicKey
- )
- )
- }
- .onEach { Timber.tag(TAG).v("$it") }
-
- private fun getTEKSignaturesForEnvironment(
- signatureListBinary: ByteArray?
- ) = TEKSignatureList
- .parseFrom(signatureListBinary)
- .signaturesList
- .asSequence()
- .onEach { Timber.tag(TAG).v(it.toString()) }
- .mapNotNull { it.signature.toByteArray() }
-}
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/sources/remote/AppConfigServerTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/sources/remote/AppConfigServerTest.kt
index f8e3b0aab..e5fffdcd3 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/sources/remote/AppConfigServerTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/sources/remote/AppConfigServerTest.kt
@@ -7,7 +7,7 @@ import de.rki.coronawarnapp.appconfig.internal.InternalConfigData
import de.rki.coronawarnapp.storage.TestSettings
import de.rki.coronawarnapp.util.CWADebug
import de.rki.coronawarnapp.util.TimeStamper
-import de.rki.coronawarnapp.util.security.VerificationKeys
+import de.rki.coronawarnapp.util.security.SignatureValidation
import io.kotest.assertions.throwables.shouldThrow
import io.kotest.matchers.shouldBe
import io.mockk.MockKAnnotations
@@ -33,7 +33,7 @@ import java.io.File
class AppConfigServerTest : BaseIOTest() {
@MockK lateinit var api: AppConfigApiV2
- @MockK lateinit var verificationKeys: VerificationKeys
+ @MockK lateinit var signatureValidation: SignatureValidation
@MockK lateinit var timeStamper: TimeStamper
@MockK lateinit var testSettings: TestSettings
private val testDir = File(IO_TEST_BASEDIR, this::class.simpleName!!)
@@ -45,7 +45,7 @@ class AppConfigServerTest : BaseIOTest() {
testDir.exists() shouldBe true
every { timeStamper.nowUTC } returns Instant.ofEpochMilli(123456789)
- every { verificationKeys.hasInvalidSignature(any(), any()) } returns false
+ every { signatureValidation.hasValidSignature(any(), any()) } returns true
mockkObject(CWADebug)
every { CWADebug.isDeviceForTestersBuild } returns false
@@ -59,7 +59,7 @@ class AppConfigServerTest : BaseIOTest() {
private fun createInstance() = AppConfigServer(
api = { api },
- verificationKeys = verificationKeys,
+ signatureValidation = signatureValidation,
timeStamper = timeStamper,
testSettings = testSettings
)
@@ -92,7 +92,7 @@ class AppConfigServerTest : BaseIOTest() {
cacheValidity = Duration.standardSeconds(123)
)
- verify(exactly = 1) { verificationKeys.hasInvalidSignature(any(), any()) }
+ verify(exactly = 1) { signatureValidation.hasValidSignature(any(), any()) }
}
@Test
@@ -113,7 +113,7 @@ class AppConfigServerTest : BaseIOTest() {
coEvery { api.getApplicationConfiguration() } returns Response.success(
APPCONFIG_BUNDLE.toResponseBody()
)
- every { verificationKeys.hasInvalidSignature(any(), any()) } returns true
+ every { signatureValidation.hasValidSignature(any(), any()) } returns false
val downloadServer = createInstance()
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/statistics/source/StatisticsServerTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/statistics/source/StatisticsServerTest.kt
index a9ce4ebd8..f1b7acf65 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/statistics/source/StatisticsServerTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/statistics/source/StatisticsServerTest.kt
@@ -1,6 +1,6 @@
package de.rki.coronawarnapp.statistics.source
-import de.rki.coronawarnapp.util.security.VerificationKeys
+import de.rki.coronawarnapp.util.security.SignatureValidation
import io.kotest.assertions.throwables.shouldThrow
import io.kotest.matchers.shouldBe
import io.mockk.MockKAnnotations
@@ -23,20 +23,20 @@ import java.io.IOException
class StatisticsServerTest : BaseIOTest() {
@MockK lateinit var api: StatisticsApiV1
- @MockK lateinit var verificationKeys: VerificationKeys
+ @MockK lateinit var signatureValidation: SignatureValidation
@MockK lateinit var cache: Cache
@BeforeEach
fun setup() {
MockKAnnotations.init(this)
- every { verificationKeys.hasInvalidSignature(any(), any()) } returns false
+ every { signatureValidation.hasValidSignature(any(), any()) } returns true
every { cache.evictAll() } just Runs
}
private fun createInstance() = StatisticsServer(
api = { api },
- verificationKeys = verificationKeys,
+ signatureValidation = signatureValidation,
cache = cache
)
@@ -49,7 +49,7 @@ class StatisticsServerTest : BaseIOTest() {
val rawStatistics = server.getRawStatistics()
rawStatistics shouldBe STATS_PROTO
- verify(exactly = 1) { verificationKeys.hasInvalidSignature(any(), any()) }
+ verify(exactly = 1) { signatureValidation.hasValidSignature(any(), any()) }
}
@Test
@@ -66,7 +66,7 @@ class StatisticsServerTest : BaseIOTest() {
@Test
fun `verification fails`() = runBlockingTest {
coEvery { api.getStatistics() } returns Response.success(STATS_ZIP.toResponseBody())
- every { verificationKeys.hasInvalidSignature(any(), any()) } returns true
+ every { signatureValidation.hasValidSignature(any(), any()) } returns false
val server = createInstance()
--
GitLab