Don't additionally base64 encode the nonce (DEV) #2410

* Don't additionally base64 encode the nonce.

* Adjust unit tests, we don't return a decoded string value and stick with ByteString.

Co-authored-by: Ralf Gehrer <ralfgehrer@users.noreply.github.com>

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/safetynet/CWASafetyNet.kt

@@ -8,11 +8,12 @@ import de.rki.coronawarnapp.datadonation.safetynet.SafetyNetException.Type
 import de.rki.coronawarnapp.main.CWASettings
 import de.rki.coronawarnapp.storage.TestSettings
 import de.rki.coronawarnapp.util.CWADebug
-import de.rki.coronawarnapp.util.HashExtensions
 import de.rki.coronawarnapp.util.HashExtensions.toSHA256
 import de.rki.coronawarnapp.util.TimeStamper
 import de.rki.coronawarnapp.util.di.AppContext
 import de.rki.coronawarnapp.util.gplay.GoogleApiVersion
+import okio.ByteString
+import okio.ByteString.Companion.decodeHex
 import okio.ByteString.Companion.toByteString
 import org.joda.time.Duration
 import org.joda.time.Instant
@@ -39,9 +40,10 @@ class CWASafetyNet @Inject constructor(
     }
 
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
-    fun calculateNonce(salt: ByteArray, payload: ByteArray): String {
+    fun calculateNonce(salt: ByteArray, payload: ByteArray): ByteString {
         val concat = salt + payload
-        return concat.toSHA256(format = HashExtensions.Format.BASE64)
+        // Default format is hex.
+        return concat.toSHA256().decodeHex()
     }
 
     override suspend fun attest(request: DeviceAttestation.Request): DeviceAttestation.Result {

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/safetynet/SafetyNetClientWrapper.kt

@@ -11,6 +11,7 @@ import de.rki.coronawarnapp.datadonation.safetynet.SafetyNetException.Type
 import de.rki.coronawarnapp.environment.EnvironmentSetup
 import kotlinx.coroutines.TimeoutCancellationException
 import kotlinx.coroutines.withTimeout
+import okio.ByteString
 import okio.ByteString.Companion.decodeBase64
 import timber.log.Timber
 import javax.inject.Inject
@@ -95,7 +96,7 @@ class SafetyNetClientWrapper @Inject constructor(
         val body: JsonObject,
         val signature: ByteArray
     ) {
-        val nonce: String? = body.get("nonce")?.asString?.decodeBase64()?.utf8()
+        val nonce: ByteString? = body.get("nonce")?.asString?.decodeBase64()
 
         val apkPackageName: String? = body.get("apkPackageName")?.asString
 

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/datadonation/safetynet/CWASafetyNetTest.kt

@@ -54,7 +54,7 @@ class CWASafetyNetTest : BaseTest() {
 
     private val defaultPayload = "Computer says no.".toByteArray()
     private val firstSalt = "LMK0jFCu/lOzl07ZHmtOqQ==".decodeBase64()!!
-    private val defaultNonce = (firstSalt.toByteArray() + defaultPayload).toSHA256(format = BASE64)
+    private val defaultNonce = (firstSalt.toByteArray() + defaultPayload).toSHA256(format = BASE64).decodeBase64()!!
 
     @BeforeEach
     fun setup() {
@@ -122,7 +122,7 @@ class CWASafetyNetTest : BaseTest() {
             salt,
             payload
         )
-        nonce shouldBe "M2EqczgxveKiptESiBNRmKqxYv5raTdzyeSZyzsCvjg="
+        nonce shouldBe "M2EqczgxveKiptESiBNRmKqxYv5raTdzyeSZyzsCvjg=".decodeBase64()
     }
 
     @Test
@@ -135,7 +135,7 @@ class CWASafetyNetTest : BaseTest() {
         otp.otp shouldBe "hello-world"
 
         val nonce = createInstance().calculateNonce(salt, payload)
-        nonce shouldBe "ANjVoDcS8v8iQdlNrcxehSggE9WZwIp7VNpjoU7cPsg="
+        nonce shouldBe "ANjVoDcS8v8iQdlNrcxehSggE9WZwIp7VNpjoU7cPsg=".decodeBase64()
     }
 
     @Test
@@ -147,7 +147,7 @@ class CWASafetyNetTest : BaseTest() {
             salt,
             payload
         )
-        nonce shouldBe "Alzb6UASmHCdnnT0M8pQv5bQ/r/+lfS/jb760+ikhxc="
+        nonce shouldBe "Alzb6UASmHCdnnT0M8pQv5bQ/r/+lfS/jb760+ikhxc=".decodeBase64()
     }
 
     @Test
@@ -160,7 +160,7 @@ class CWASafetyNetTest : BaseTest() {
         ppa.exposureRiskMetadataSetList.first().riskLevel shouldBe PpaData.PPARiskLevel.RISK_LEVEL_HIGH
 
         val nonce = createInstance().calculateNonce(salt, payload)
-        nonce shouldBe "bd6kMfLKby3pzEqW8go1ZgmHN/bU1p/4KG6+1GeB288="
+        nonce shouldBe "bd6kMfLKby3pzEqW8go1ZgmHN/bU1p/4KG6+1GeB288=".decodeBase64()
     }
 
     @Test
@@ -168,7 +168,7 @@ class CWASafetyNetTest : BaseTest() {
         val payload = "Computer says no.".toByteArray()
         val salt = "Don't be so salty".toByteArray()
         val nonce = createInstance().calculateNonce(salt, payload)
-        nonce shouldBe (salt + payload).toSHA256(format = BASE64)
+        nonce shouldBe (salt + payload).toSHA256(format = BASE64).decodeBase64()
     }
 
     @Test
@@ -198,7 +198,7 @@ class CWASafetyNetTest : BaseTest() {
 
     @Test
     fun `request nonce must match response nonce`() = runBlockingTest {
-        every { clientReport.nonce } returns "missmatch"
+        every { clientReport.nonce } returns "missmatch".decodeBase64()
         val exception = shouldThrow<SafetyNetException> {
             createInstance().attest(TestAttestationRequest("Computer says no.".toByteArray()))
         }

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/datadonation/safetynet/SafetyNetClientWrapperTest.kt

@@ -148,7 +148,7 @@ class SafetyNetClientWrapperTest : BaseTest() {
                 body shouldBe JsonParser.parseString(JWS_BODY)
                 signature shouldBe JWS_SIGNATURE_BASE64.decodeBase64()!!.toByteArray()
 
-                nonce shouldBe "AAAAAAAAAAAAAAAAAAAAAA==".decodeBase64()?.utf8()
+                nonce shouldBe "AAAAAAAAAAAAAAAAAAAAAA==".decodeBase64()
                 apkPackageName shouldBe "de.rki.coronawarnapp.test"
                 basicIntegrity shouldBe false
                 ctsProfileMatch shouldBe false
@@ -164,7 +164,7 @@ class SafetyNetClientWrapperTest : BaseTest() {
             createInstance().attest("hodl".toByteArray()).apply {
                 body shouldBe JsonParser.parseString(JWS_BODY_MINIMAL)
 
-                nonce shouldBe "AAAAAAAAAAAAAAAAAAAAAA==".decodeBase64()?.utf8()
+                nonce shouldBe "AAAAAAAAAAAAAAAAAAAAAA==".decodeBase64()
                 apkPackageName shouldBe "de.rki.coronawarnapp.test"
                 basicIntegrity shouldBe false
                 ctsProfileMatch shouldBe false