diff --git a/res/values/strings.xml b/res/values/strings.xml index 09b6fc2c..9cf1dec3 100644 --- a/res/values/strings.xml +++ b/res/values/strings.xml @@ -176,6 +176,7 @@ SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted Device not reachable Device already paired Timed out + Device clocks are out of sync Canceled by user Canceled by other peer Encryption Info diff --git a/src/org/kde/kdeconnect/Device.kt b/src/org/kde/kdeconnect/Device.kt index b9965dd0..3ed3a76c 100644 --- a/src/org/kde/kdeconnect/Device.kt +++ b/src/org/kde/kdeconnect/Device.kt @@ -159,19 +159,18 @@ class Device : PacketReceiver { val certificate: Certificate get() = deviceInfo.certificate + val verificationKey: String? + get() = pairingHandler.verificationKey() + // Returns 0 if the version matches, < 0 if it is older or > 0 if it is newer fun compareProtocolVersion(): Int = deviceInfo.protocolVersion - DeviceHelper.ProtocolVersion - val isPaired: Boolean get() = pairingHandler.state == PairingHandler.PairState.Paired - val isPairRequested: Boolean - get() = pairingHandler.state == PairingHandler.PairState.Requested - - val isPairRequestedByPeer: Boolean - get() = pairingHandler.state == PairingHandler.PairState.RequestedByPeer + val pairStatus : PairingHandler.PairState + get() = pairingHandler.state fun addPairingCallback(callback: PairingCallback) = pairingCallbacks.add(callback) @@ -289,8 +288,6 @@ class Device : PacketReceiver { val notificationManager = ContextCompat.getSystemService(context, NotificationManager::class.java)!! - val verificationKey = SslHelper.getVerificationKey(SslHelper.certificate, deviceInfo.certificate) - val noti = NotificationCompat.Builder(context, NotificationHelper.Channels.DEFAULT) .setContentTitle(res.getString(R.string.pairing_request_from, name)) .setContentText(res.getString(R.string.pairing_verification_code, verificationKey)) diff --git a/src/org/kde/kdeconnect/Helpers/SecurityHelpers/SslHelper.java b/src/org/kde/kdeconnect/Helpers/SecurityHelpers/SslHelper.java index 56285a20..15339f6a 100644 --- a/src/org/kde/kdeconnect/Helpers/SecurityHelpers/SslHelper.java +++ b/src/org/kde/kdeconnect/Helpers/SecurityHelpers/SslHelper.java @@ -282,31 +282,4 @@ public class SslHelper { return IETFUtils.valueToString(rdn.getFirst().getValue()); } - public static String getVerificationKey(Certificate certificateA, Certificate certificateB) { - try { - byte[] a = certificateA.getPublicKey().getEncoded(); - byte[] b = certificateB.getPublicKey().getEncoded(); - - if (Arrays.compareUnsigned(a, b) < 0) { - // Swap them so on both devices they are in the same order - byte[] aux = a; - a = b; - b = aux; - } - - byte[] concat = new byte[a.length + b.length]; - System.arraycopy(a, 0, concat, 0, a.length); - System.arraycopy(b, 0, concat, a.length, b.length); - - byte[] hash = MessageDigest.getInstance("SHA-256").digest(concat); - Formatter formatter = new Formatter(); - for (byte value : hash) { - formatter.format("%02x", value); - } - return formatter.toString().substring(0,8).toUpperCase(Locale.ROOT); - } catch(Exception e) { - e.printStackTrace(); - return "error"; - } - } } diff --git a/src/org/kde/kdeconnect/PairingHandler.kt b/src/org/kde/kdeconnect/PairingHandler.kt index 6373e3bf..e00ed0cf 100644 --- a/src/org/kde/kdeconnect/PairingHandler.kt +++ b/src/org/kde/kdeconnect/PairingHandler.kt @@ -8,7 +8,13 @@ package org.kde.kdeconnect import android.util.Log import androidx.annotation.VisibleForTesting import kotlinx.coroutines.* +import org.bouncycastle.util.Arrays +import org.kde.kdeconnect.Helpers.SecurityHelpers.SslHelper import org.kde.kdeconnect_tp.R +import java.security.MessageDigest +import java.security.cert.Certificate +import java.util.Formatter +import kotlin.math.abs import kotlin.time.Duration.Companion.seconds class PairingHandler(private val device: Device, private val callback: PairingCallback, var state: PairState) { @@ -31,6 +37,7 @@ class PairingHandler(private val device: Device, private val callback: PairingCa private val pairingJob = SupervisorJob() private val pairingScope = CoroutineScope(Dispatchers.IO + pairingJob) + private var pairingTimestamp = 0L fun packetReceived(np: NetworkPacket) { cancelTimer() @@ -50,10 +57,26 @@ class PairingHandler(private val device: Device, private val callback: PairingCa Log.w("PairingHandler", "Received pairing request from a device we already trusted.") // It would be nice to auto-accept the pairing request here, but since the pairing accept and pairing request // messages are identical, this could create an infinite loop if both devices are "accepting" each other pairs. - // Instead, unpair and handle as if "NotPaired". + // Instead, unpair and handle as if "NotPaired". TODO: No longer true in protocol version 8 state = PairState.NotPaired callback.unpaired() } + + if (device.protocolVersion >= 8) { + pairingTimestamp = np.getLong("timestamp", -1L) + if (pairingTimestamp == -1L) { + state = PairState.NotPaired + callback.unpaired() + return + } + val currentTimestamp = System.currentTimeMillis() / 1000L + if (abs(pairingTimestamp - currentTimestamp) > allowedTimestampDifferenceSeconds) { + state = PairState.NotPaired + callback.pairingFailed(device.context.getString(R.string.error_clocks_not_match)) + return + } + } + state = PairState.RequestedByPeer pairingScope.launch { @@ -85,6 +108,18 @@ class PairingHandler(private val device: Device, private val callback: PairingCa } } + fun verificationKey(): String? { + return if (device.protocolVersion >= 8) { + if (state != PairState.Requested && state != PairState.RequestedByPeer) { + return null + } else { + getVerificationKey(SslHelper.certificate, device.certificate, pairingTimestamp) + } + } else { + getVerificationKeyV7(SslHelper.certificate, device.certificate) + } + } + fun requestPairing() { cancelTimer() @@ -126,6 +161,8 @@ class PairingHandler(private val device: Device, private val callback: PairingCa } val np = NetworkPacket(NetworkPacket.PACKET_TYPE_PAIR) np["pair"] = true + pairingTimestamp = System.currentTimeMillis() / 1000L + np["timestamp"] = pairingTimestamp device.sendPacket(np, statusCallback) } @@ -181,4 +218,36 @@ class PairingHandler(private val device: Device, private val callback: PairingCa private fun cancelTimer() { pairingJob.cancelChildren() } + + companion object { + private const val allowedTimestampDifferenceSeconds = 1_800 // 30 minutes + + // Concatenate in a deterministic order so on both devices the result is the same + private fun sortedConcat(a: ByteArray, b: ByteArray): ByteArray { + return if (Arrays.compareUnsigned(a, b) < 0) { + b + a + } else { + a + b + } + } + + private fun humanReadableHash(bytes: ByteArray): String { + val hash = MessageDigest.getInstance("SHA-256").digest(bytes) + val formatter = Formatter() + for (value in hash) { + formatter.format("%02x", value) + } + return formatter.toString().substring(0, 8).uppercase() + } + fun getVerificationKey(certificateA: Certificate, certificateB: Certificate, timestamp: Long): String { + val certsConcat = sortedConcat(certificateA.publicKey.encoded, certificateB.publicKey.encoded) + return humanReadableHash(certsConcat + timestamp.toString().toByteArray()) + } + + fun getVerificationKeyV7(certificateA: Certificate, certificateB: Certificate): String { + val certsConcat = sortedConcat(certificateA.publicKey.encoded, certificateB.publicKey.encoded) + return humanReadableHash(certsConcat) + } + } + } diff --git a/src/org/kde/kdeconnect/UserInterface/DeviceFragment.kt b/src/org/kde/kdeconnect/UserInterface/DeviceFragment.kt index bd69a658..0929dbdb 100644 --- a/src/org/kde/kdeconnect/UserInterface/DeviceFragment.kt +++ b/src/org/kde/kdeconnect/UserInterface/DeviceFragment.kt @@ -115,16 +115,9 @@ class DeviceFragment : Fragment() { this.refreshDevicesAction() } - requirePairingBinding().pairVerification.text = SslHelper.getVerificationKey(SslHelper.certificate, device?.certificate) - requirePairingBinding().pairButton.setOnClickListener { - with(requirePairingBinding()) { - pairButton.visibility = View.GONE - pairMessage.text = getString(R.string.pair_requested) - pairProgress.visibility = View.VISIBLE - } device?.requestPairing() - mActivity?.invalidateOptionsMenu() + refreshUI() } requirePairingBinding().acceptButton.setOnClickListener { device?.apply { @@ -242,7 +235,7 @@ class DeviceFragment : Fragment() { true } } - if (device.isPairRequested) { + if (device.pairStatus == PairingHandler.PairState.Requested) { menu.add(R.string.cancel_pairing).setOnMenuItemClickListener { device.cancelPairing() true @@ -275,19 +268,36 @@ class DeviceFragment : Fragment() { //Once in-app, there is no point in keep displaying the notification if any device.hidePairingNotification() - if (device.isPairRequestedByPeer) { - with (requirePairingBinding()) { - pairMessage.setText(R.string.pair_requested) - pairVerification.visibility = View.VISIBLE - pairVerification.text = SslHelper.getVerificationKey(SslHelper.certificate, device.certificate) - pairingButtons.visibility = View.VISIBLE - pairProgress.visibility = View.GONE - pairButton.visibility = View.GONE - pairRequestButtons.visibility = View.VISIBLE + when (device.pairStatus) { + PairingHandler.PairState.NotPaired -> { + requireErrorBinding().errorMessageContainer.visibility = View.GONE + requireDeviceBinding().deviceView.visibility = View.GONE + requirePairingBinding().pairingButtons.visibility = View.VISIBLE + requirePairingBinding().pairVerification.visibility = View.GONE } - requireDeviceBinding().deviceView.visibility = View.GONE - } else { - if (device.isPaired) { + PairingHandler.PairState.Requested -> { + with(requirePairingBinding()) { + pairButton.visibility = View.GONE + pairMessage.text = getString(R.string.pair_requested) + pairProgress.visibility = View.VISIBLE + pairVerification.text = device.verificationKey + pairVerification.visibility = View.VISIBLE + } + } + PairingHandler.PairState.RequestedByPeer -> { + with (requirePairingBinding()) { + pairMessage.setText(R.string.pair_requested) + pairVerification.visibility = View.VISIBLE + pairingButtons.visibility = View.VISIBLE + pairProgress.visibility = View.GONE + pairButton.visibility = View.GONE + pairRequestButtons.visibility = View.VISIBLE + pairVerification.text = device.verificationKey + pairVerification.visibility = View.VISIBLE + } + requireDeviceBinding().deviceView.visibility = View.GONE + } + PairingHandler.PairState.Paired -> { requirePairingBinding().pairingButtons.visibility = View.GONE if (device.isReachable) { val context = requireContext() @@ -305,13 +315,9 @@ class DeviceFragment : Fragment() { requireErrorBinding().errorMessageContainer.visibility = View.VISIBLE requireDeviceBinding().deviceView.visibility = View.GONE } - } else { - requireErrorBinding().errorMessageContainer.visibility = View.GONE - requireDeviceBinding().deviceView.visibility = View.GONE - requirePairingBinding().pairingButtons.visibility = View.VISIBLE } - mActivity?.invalidateOptionsMenu() } + mActivity?.invalidateOptionsMenu() } private val pairingCallback: PairingHandler.PairingCallback = object : PairingHandler.PairingCallback { diff --git a/tests/org/kde/kdeconnect/Helpers/SslHelperTest.kt b/tests/org/kde/kdeconnect/Helpers/SslHelperTest.kt index e529e4ff..565e2359 100644 --- a/tests/org/kde/kdeconnect/Helpers/SslHelperTest.kt +++ b/tests/org/kde/kdeconnect/Helpers/SslHelperTest.kt @@ -12,6 +12,7 @@ import org.junit.Before import org.junit.Test import org.kde.kdeconnect.Helpers.SecurityHelpers.SslHelper import org.kde.kdeconnect.MockSharedPreference +import org.kde.kdeconnect.PairingHandler import org.mockito.ArgumentMatchers import org.mockito.MockedStatic import org.mockito.Mockito @@ -22,7 +23,6 @@ import java.util.Base64 class SSLHelperTest { private lateinit var context: Context private lateinit var sharedPreferences: MockSharedPreference - /** Certificate captured from debug session */ private val certificateBase64 = "MIIBkzCCATmgAwIBAgIBATAKBggqhkjOPQQDBDBTMS0wKwYDVQQDDCRlZTA2MWE3NV9lNDAzXzRlY2NfOTI2MV81ZmZlMjcyMmY2OTgxFDASBgNVBAsMC0tERSBDb25uZWN0MQwwCgYDVQQKDANLREUwHhcNMjMwOTE1MjIwMDAwWhcNMzQwOTE1MjIwMDAwWjBTMS0wKwYDVQQDDCRlZTA2MWE3NV9lNDAzXzRlY2NfOTI2MV81ZmZlMjcyMmY2OTgxFDASBgNVBAsMC0tERSBDb25uZWN0MQwwCgYDVQQKDANLREUwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAASqOIKTm5j6x8DKgYSkItLmjCgIXP0gkOW6bmVvloDGsYnvqYLMFGe7YW8g8lT/qPBTEfDOM4UpQ8X6jidE+XrnMAoGCCqGSM49BAMEA0gAMEUCIEpk6VNpbt3tfbWDf0TmoJftRq3wAs3Dke7d5vMZlivyAiEA/ZXtSRqPjs/2RN9SynKhSUA9/z0PNq6LYoAaC6TdomM=" private val certificateHash = "fc:1f:b3:d3:d3:3b:23:42:e4:5c:74:b1:a6:13:dc:df:e5:e1:f0:29:d6:68:24:9f:50:49:52:a9:a8:04:1e:31:" private val deviceId = "testDevice" @@ -40,13 +40,13 @@ class SSLHelperTest { mockBase64.`when` { android.util.Base64.encodeToString(ArgumentMatchers.any(ByteArray::class.java), ArgumentMatchers.anyInt()) }.thenAnswer { invocation: InvocationOnMock -> - java.util.Base64.getMimeEncoder().encodeToString(invocation.arguments[0] as ByteArray) + Base64.getMimeEncoder().encodeToString(invocation.arguments[0] as ByteArray) } mockBase64.`when` { android.util.Base64.decode(ArgumentMatchers.anyString(), ArgumentMatchers.anyInt()) }.thenAnswer { invocation: InvocationOnMock -> - java.util.Base64.getMimeDecoder().decode(invocation.arguments[0] as String) + Base64.getMimeDecoder().decode(invocation.arguments[0] as String) } this.mockBase64 = mockBase64 @@ -112,14 +112,4 @@ class SSLHelperTest { Assert.assertEquals(expected, commonName) } - @Test - fun getVerificationKey() { - sharedPreferences.edit().putString(certificateKey, certificateBase64).apply() - val cert = SslHelper.getDeviceCertificate(context, deviceId) - - // Normally not used with same certificate, but it's fine for testing - val verificationKey = SslHelper.getVerificationKey(cert, cert) - val expected = "97A75917" - Assert.assertEquals(expected, verificationKey) - } -} \ No newline at end of file +} diff --git a/tests/org/kde/kdeconnect/PairingHandlerTest.kt b/tests/org/kde/kdeconnect/PairingHandlerTest.kt new file mode 100644 index 00000000..572d2f50 --- /dev/null +++ b/tests/org/kde/kdeconnect/PairingHandlerTest.kt @@ -0,0 +1,47 @@ +/* + * SPDX-FileCopyrightText: 2025 Albert Vaca Cintora + * + * SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted-GPL + */ +package org.kde.kdeconnect + +import org.junit.Assert +import org.junit.Test +import org.kde.kdeconnect.Helpers.SecurityHelpers.SslHelper +import java.util.Base64 + +class PairingHandlerTest { + private val certA = SslHelper.parseCertificate(Base64.getMimeDecoder().decode( + "MIIBkzCCATmgAwIBAgIBATAKBggqhkjOPQQDBDBTMS0wKwYDVQQDDCRlZTA2MWE3NV9lNDAzXzRlY2NfOTI2" + + "MV81ZmZlMjcyMmY2OTgxFDASBgNVBAsMC0tERSBDb25uZWN0MQwwCgYDVQQKDANLREUwHhcNMjMwOTE1MjIw" + + "MDAwWhcNMzQwOTE1MjIwMDAwWjBTMS0wKwYDVQQDDCRlZTA2MWE3NV9lNDAzXzRlY2NfOTI2MV81ZmZlMjcy" + + "MmY2OTgxFDASBgNVBAsMC0tERSBDb25uZWN0MQwwCgYDVQQKDANLREUwWTATBgcqhkjOPQIBBggqhkjOPQMB" + + "BwNCAASqOIKTm5j6x8DKgYSkItLmjCgIXP0gkOW6bmVvloDGsYnvqYLMFGe7YW8g8lT/qPBTEfDOM4UpQ8X6" + + "jidE+XrnMAoGCCqGSM49BAMEA0gAMEUCIEpk6VNpbt3tfbWDf0TmoJftRq3wAs3Dke7d5vMZlivyAiEA/ZXt" + + "SRqPjs/2RN9SynKhSUA9/z0PNq6LYoAaC6TdomM=" + )) + private val certB = SslHelper.parseCertificate(Base64.getMimeDecoder().decode( + "MIIBkzCCATmgAwIBAgIBATAKBggqhkjOPQQDBDBTMS0wKwYDVQQDDCQxNTdiYmMyOF82ZjJiXzRiMTZfYmQw" + + "Ml8xMzM0NWMwMjU0M2MxFDASBgNVBAsMC0tERSBDb25uZWN0MQwwCgYDVQQKDANLREUwHhcNMjQwMTE3MjMw" + + "MDAwWhcNMzUwMTE3MjMwMDAwWjBTMS0wKwYDVQQDDCQxNTdiYmMyOF82ZjJiXzRiMTZfYmQwMl8xMzM0NWMw" + + "MjU0M2MxFDASBgNVBAsMC0tERSBDb25uZWN0MQwwCgYDVQQKDANLREUwWTATBgcqhkjOPQIBBggqhkjOPQMB" + + "BwNCAAQ5W53rrDJps9v/sszQf0eLtvoGiRbfsY+snO6IJJfi1pFeHDQj2nAE+aTyUYelrcx1eIuqxFHnJTFt" + + "/HqXwuAvMAoGCCqGSM49BAMEA0gAMEUCIBIk3zKPz/M0c82nvCGFDXGGmfdojHsx3G5DbYNNKqFVAiEAzhBG" + + "e960/4NDiaVcOplBaeg5xNJKs3Kq+22J6JOii4Y=")) + + @Test + fun getVerificationKey() { + val timestampA = 1737228658L + val timestampB = 2737228658L + Assert.assertEquals("54DC916E", PairingHandler.getVerificationKey(certA, certB, timestampA)) + Assert.assertEquals("54DC916E", PairingHandler.getVerificationKey(certB, certA, timestampA)) + Assert.assertEquals("8C07153A", PairingHandler.getVerificationKey(certA, certB, timestampB)) + } + + @Test + fun getVerificationKeyV7() { + Assert.assertEquals("F3900DB5", PairingHandler.getVerificationKeyV7(certA, certB)) + Assert.assertEquals("F3900DB5", PairingHandler.getVerificationKeyV7(certB, certA)) + Assert.assertEquals("97A75917", PairingHandler.getVerificationKeyV7(certA, certA)) + } +}