From a28dc377aebbff3b60f131117a0cea77397cb206 Mon Sep 17 00:00:00 2001 From: James Rich <2199651+jamesarich@users.noreply.github.com> Date: Thu, 27 Mar 2025 15:58:43 -0500 Subject: [PATCH] fix: Handle node public key mismatch and show warning (#1720) * Handle node public key mismatch and show warning - Add a mismatchKey flag to Node and MessageTopBar to indicate a public key mismatch. - Set the public key to a default error value (all zeros) when a node's public key changes. - Display a warning in the MessageTopBar when a key mismatch is detected in PKC. - Only clear all nodes when a different mynode number is present. * feat: Add key mismatch detection to NodeInfoDao This commit introduces a new feature to the `NodeInfoDao` that detects and handles public key mismatches for existing nodes. - A new function `upsertCheckKeyMatch` is added to `NodeInfoDao` that checks for public key changes when upserting a node. If a mismatch is detected, the public key is set to `ERROR_BYTE_STRING`, and a warning is logged. - The function `upsertCheckKeyMatch` is used instead of `upsert` in `NodeRepository` and in `putAll` inside of `NodeInfoDao`. - A new test `testPkcMismatch` is added to `NodeInfoDaoTest` to verify the key mismatch detection. - Changed `testNodes` to have unique public keys. - Added `mismatchKey` state to the node model. * detekt spacing * Refactor: Correctly handle different node installations in NodeRepository The logic for detecting different node installations in `NodeRepository.kt` was inverted, this commit fixes the logic to use `!=` instead of `==` to detect if the node number has changed. --- .../com/geeksville/mesh/NodeInfoDaoTest.kt | 14 ++++++++++- .../mesh/database/NodeRepository.kt | 7 ++++-- .../mesh/database/dao/NodeInfoDao.kt | 23 ++++++++++++++++--- .../mesh/database/entity/NodeEntity.kt | 3 ++- .../java/com/geeksville/mesh/model/Node.kt | 5 ++-- .../com/geeksville/mesh/ui/message/Message.kt | 7 ++++-- 6 files changed, 47 insertions(+), 12 deletions(-) diff --git a/app/src/androidTest/java/com/geeksville/mesh/NodeInfoDaoTest.kt b/app/src/androidTest/java/com/geeksville/mesh/NodeInfoDaoTest.kt index 445e9534..737685f3 100644 --- a/app/src/androidTest/java/com/geeksville/mesh/NodeInfoDaoTest.kt +++ b/app/src/androidTest/java/com/geeksville/mesh/NodeInfoDaoTest.kt @@ -26,6 +26,7 @@ import com.geeksville.mesh.database.entity.MyNodeEntity import com.geeksville.mesh.database.entity.NodeEntity import com.geeksville.mesh.model.Node import com.geeksville.mesh.model.NodeSortOption +import com.google.protobuf.ByteString import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.map import kotlinx.coroutines.runBlocking @@ -92,7 +93,6 @@ class NodeInfoDaoTest { 41.878113 to -87.629799, // Chicago 39.952583 to -75.165222, // Philadelphia ) - private val testNodes = listOf(ourNode, unknownNode) + testPositions.mapIndexed { index, pos -> NodeEntity( num = 9 + index, @@ -102,6 +102,7 @@ class NodeInfoDaoTest { shortName = "KM$index" hwModel = MeshProtos.HardwareModel.ANDROID_SIM isLicensed = false + publicKey = ByteString.copyFrom(ByteArray(32) { index.toByte() }) }, longName = "Kevin Mester$index", shortName = "KM$index", latitude = pos.first, longitude = pos.second, @@ -203,4 +204,15 @@ class NodeInfoDaoTest { val containsUnsetNode = nodes.any { it.isUnknownUser } assertTrue(containsUnsetNode) } + + @Test + fun testPkcMismatch() = runBlocking { + val newNode = testNodes[1].copy(user = testNodes[1].user.copy { + publicKey = ByteString.copyFrom(ByteArray(32) { 99 }) + }) + nodeInfoDao.putAll(listOf(newNode)) + val nodes = getNodes() + val containsMismatchNode = nodes.any { it.mismatchKey } + assertTrue(containsMismatchNode) + } } diff --git a/app/src/main/java/com/geeksville/mesh/database/NodeRepository.kt b/app/src/main/java/com/geeksville/mesh/database/NodeRepository.kt index ff351359..eef931d4 100644 --- a/app/src/main/java/com/geeksville/mesh/database/NodeRepository.kt +++ b/app/src/main/java/com/geeksville/mesh/database/NodeRepository.kt @@ -103,13 +103,16 @@ class NodeRepository @Inject constructor( ).mapLatest { list -> list.map { it.toModel() } }.flowOn(dispatchers.io).conflate() suspend fun upsert(node: NodeEntity) = withContext(dispatchers.io) { - nodeInfoDao.upsert(node) + nodeInfoDao.upsertCheckKeyMatch(node) } suspend fun installNodeDB(mi: MyNodeEntity, nodes: List) = withContext(dispatchers.io) { + val isDifferentNode = myNodeInfo.value?.myNodeNum != mi.myNodeNum nodeInfoDao.clearMyNodeInfo() nodeInfoDao.setMyNodeInfo(mi) // set MyNodeEntity first - nodeInfoDao.clearNodeInfo() + if (isDifferentNode) { + nodeInfoDao.clearNodeInfo() + } nodeInfoDao.putAll(nodes) } diff --git a/app/src/main/java/com/geeksville/mesh/database/dao/NodeInfoDao.kt b/app/src/main/java/com/geeksville/mesh/database/dao/NodeInfoDao.kt index 5d7e9239..0f41091f 100644 --- a/app/src/main/java/com/geeksville/mesh/database/dao/NodeInfoDao.kt +++ b/app/src/main/java/com/geeksville/mesh/database/dao/NodeInfoDao.kt @@ -17,6 +17,7 @@ package com.geeksville.mesh.database.dao +import android.util.Log import androidx.room.Dao import androidx.room.Insert import androidx.room.MapColumn @@ -26,10 +27,11 @@ import androidx.room.Transaction import androidx.room.Upsert import com.geeksville.mesh.database.entity.MetadataEntity import com.geeksville.mesh.database.entity.MyNodeEntity -import com.geeksville.mesh.database.entity.NodeWithRelations import com.geeksville.mesh.database.entity.NodeEntity +import com.geeksville.mesh.database.entity.NodeWithRelations import kotlinx.coroutines.flow.Flow +private const val TAG = "NodeInfoDao" @Suppress("TooManyFunctions") @Dao interface NodeInfoDao { @@ -108,8 +110,20 @@ interface NodeInfoDao { @Upsert fun upsert(node: NodeEntity) - @Insert(onConflict = OnConflictStrategy.REPLACE) - fun putAll(nodes: List) + fun upsertCheckKeyMatch(node: NodeEntity) { + val existingNode = getNodeByNum(node.num) + if (existingNode != null && existingNode.user.publicKey != node.user.publicKey) { + Log.w(TAG, "Node ${node.num} has changed its public key") + val user = + node.user.toBuilder().setPublicKey(NodeEntity.ERROR_BYTE_STRING).build() + node.user = user + } + upsert(node) + } + @Transaction + fun putAll(nodes: List) { + nodes.forEach { upsertCheckKeyMatch(it) } + } @Query("DELETE FROM nodes") fun clearNodeInfo() @@ -122,4 +136,7 @@ interface NodeInfoDao { @Query("DELETE FROM metadata WHERE num=:num") fun deleteMetadata(num: Int) + + @Query("SELECT * FROM nodes WHERE num = :num") + fun getNodeByNum(num: Int): NodeEntity? } diff --git a/app/src/main/java/com/geeksville/mesh/database/entity/NodeEntity.kt b/app/src/main/java/com/geeksville/mesh/database/entity/NodeEntity.kt index 58e90993..5a04f507 100644 --- a/app/src/main/java/com/geeksville/mesh/database/entity/NodeEntity.kt +++ b/app/src/main/java/com/geeksville/mesh/database/entity/NodeEntity.kt @@ -151,7 +151,7 @@ data class NodeEntity( val isUnknownUser get() = user.hwModel == MeshProtos.HardwareModel.UNSET val hasPKC get() = !user.publicKey.isEmpty - val errorByteString: ByteString get() = ByteString.copyFrom(ByteArray(32) { 0 }) + val errorByteString: ByteString get() = ERROR_BYTE_STRING fun setPosition(p: MeshProtos.Position, defaultTime: Int = currentTime()) { position = p.copy { time = if (p.time != 0) p.time else defaultTime } @@ -174,6 +174,7 @@ data class NodeEntity( fun degD(i: Int) = i * 1e-7 fun degI(d: Double) = (d * 1e7).toInt() + val ERROR_BYTE_STRING: ByteString = ByteString.copyFrom(ByteArray(32) { 0 }) fun currentTime() = (System.currentTimeMillis() / 1000).toInt() } diff --git a/app/src/main/java/com/geeksville/mesh/model/Node.kt b/app/src/main/java/com/geeksville/mesh/model/Node.kt index 30bff207..ccd15538 100644 --- a/app/src/main/java/com/geeksville/mesh/model/Node.kt +++ b/app/src/main/java/com/geeksville/mesh/model/Node.kt @@ -24,10 +24,10 @@ import com.geeksville.mesh.PaxcountProtos import com.geeksville.mesh.TelemetryProtos.DeviceMetrics import com.geeksville.mesh.TelemetryProtos.EnvironmentMetrics import com.geeksville.mesh.TelemetryProtos.PowerMetrics +import com.geeksville.mesh.database.entity.NodeEntity import com.geeksville.mesh.util.GPSFormat import com.geeksville.mesh.util.latLongToMeter import com.geeksville.mesh.util.toDistanceString -import com.google.protobuf.ByteString @Suppress("MagicNumber") data class Node( @@ -59,8 +59,7 @@ data class Node( val isUnknownUser get() = user.hwModel == MeshProtos.HardwareModel.UNSET val hasPKC get() = !user.publicKey.isEmpty - val errorByteString: ByteString get() = ByteString.copyFrom(ByteArray(32) { 0 }) - val mismatchKey get() = user.publicKey == errorByteString + val mismatchKey get() = user.publicKey == NodeEntity.ERROR_BYTE_STRING val hasEnvironmentMetrics: Boolean get() = environmentMetrics != EnvironmentMetrics.getDefaultInstance() diff --git a/app/src/main/java/com/geeksville/mesh/ui/message/Message.kt b/app/src/main/java/com/geeksville/mesh/ui/message/Message.kt index 95a89806..271ba71f 100644 --- a/app/src/main/java/com/geeksville/mesh/ui/message/Message.kt +++ b/app/src/main/java/com/geeksville/mesh/ui/message/Message.kt @@ -184,6 +184,8 @@ internal fun MessageScreen( DataPacket.ID_BROADCAST -> channelName else -> viewModel.getUser(nodeId).longName } + val mismatchKey = + DataPacket.PKC_CHANNEL_INDEX == channelIndex && viewModel.getNode(nodeId).mismatchKey // if (channelIndex != DataPacket.PKC_CHANNEL_INDEX && nodeId != DataPacket.ID_BROADCAST) { // subtitle = "(ch: $channelIndex - $channelName)" @@ -242,7 +244,7 @@ internal fun MessageScreen( } } } else { - MessageTopBar(title, channelIndex, onNavigateBack) + MessageTopBar(title, channelIndex, mismatchKey, onNavigateBack) } }, bottomBar = { @@ -368,6 +370,7 @@ private fun ActionModeTopBar( private fun MessageTopBar( title: String, channelIndex: Int?, + mismatchKey: Boolean = false, onNavigateBack: () -> Unit ) = TopAppBar( title = { Text(text = title) }, @@ -381,7 +384,7 @@ private fun MessageTopBar( }, actions = { if (channelIndex == DataPacket.PKC_CHANNEL_INDEX) { - NodeKeyStatusIcon(hasPKC = true, mismatchKey = false) + NodeKeyStatusIcon(hasPKC = true, mismatchKey = mismatchKey) } } )