Skip to content

Conversation

@GUVWAF
Copy link
Member

@GUVWAF GUVWAF commented Oct 12, 2025

For consideration to improve the UX when PKI fails after a remote node changed their keys (meshtastic/firmware#8211).

The idea is that when we receive a NodeInfo with a different public key than the device has currently stored in its NodeDB, we do not simply ignore it like currently, but we inform the client about this. The client could then either already ask the user if it would like to accept this different key (by importing the same node as Shared Contact with new key), or cache this different public key to be used when the node is receiving a PKI encrypted packet it couldn't decrypt (this is the less "spammy" approach). If it did not yet receive a different public key when the latter happens, it could also ask the user if it wants to clear the currently stored key, such that it is able to accept a new one.

@jp-bennett
Copy link
Contributor

I want to be very careful about this approach, as we still need to hash out whether we're going to start requiring nodenum to be derived from the public key.

This could also be useful for things like key verification over the mesh, as a way to verify a previously unknown key.

@GUVWAF
Copy link
Member Author

GUVWAF commented Oct 12, 2025

I want to be very careful about this approach, as we still need to hash out whether we're going to start requiring nodenum to be derived from the public key.

That's a breaking change though, so I presume that would still take some time. IMO this PR is a pretty easy UX improvement without any invasive changes. If we don't need it anymore later on, it's easy to deprecate.

@shalberd
Copy link

we're going to start requiring nodenum to be derived from the public key

Hmm, I don't know about that. A key is just that, randomly-generated private / public key. I know the context of signing something via the private key, to validate that something is legit.
Isn't node number a kind of a MAC hardware address / number, as compared to user id (meshtastic !2e2fa)? Why derive either of the two from a software-generated key / signature?

@GUVWAF
Copy link
Member Author

GUVWAF commented Oct 19, 2025

@garthvh @jamesarich I would like to get your views as well. Is this something that’s reasonable to implement in the apps?

@jamesarich
Copy link
Contributor

@GUVWAF - At first glance, this appears to put a lot of interaction, logic, and required user decision around reciept of a nodeinfo packet - which currently requires zero user interaction.

One of our current client goals is to improve user experience. Adding more error warnings/dialogs/interactions/instructions/decisions that may have complex security language is not really a step in that direction.

@GUVWAF
Copy link
Member Author

GUVWAF commented Oct 19, 2025

@jamesarich That's why I would advocate for using the "non-spammy" approach; only when someone tries to DM you and the public key doesn't match, show the notification. The first "DifferentPublicKey" notification is purely for the client itself to cache the different public key.

One of our current client goals is to improve user experience.

IMO this would greatly improve the user experience. Currently there is no way for the receiving node to know someone tried to DM them, but it couldn't be decrypted. Issues like meshtastic/firmware#8211 and meshtastic/firmware#8321 reflect this.

We can discuss on the wording itself, but it doesn't have to be complex security language. Just something like "Node x tried to send you a message with a new encryption key, do you want to trust this new key?".

@jamesarich
Copy link
Contributor

jamesarich commented Oct 19, 2025

As we wait for @garthvh to weigh in too, I'm curious how client implementation would work here..
When we recieve a dm from a someone whose public key does not match was currently stored - Do we:

  • store an additional field on the old nodeinfo/entity, DifferentPublicKey, only updating the PublicKey field on user consent
    OR
  • store an additional entire new nodeinfo/entity (which may include other changed fields) - and swap entirely on user consent (this would require some extra thought as currently node entries are keyed off of the nodenum in the client database)

@jamesarich
Copy link
Contributor

Also at what point do we notify the user that this happened? It seems like it would need an immediate additional notification/interaction when the issue occurs, to allow the user to resolve/receive/decode the direct message.

@GUVWAF
Copy link
Member Author

GUVWAF commented Oct 19, 2025

I think indeed option 2 would be preferred, as we also do not update the other fields of the NodeInfo when the public key changed. However, for simplicity, option 1 would be fine as those fields would be updated on the next received NodeInfo anyway.

It seems like it would need an immediate additional notification/interaction when the issue occurs, to allow the user to resolve/receive/decode the direct message.

I think the original direct message that triggered the notification would still be considered lost, as the firmware will indeed just drop it immediately. But at least subsequent packets can be received again, and you can ask for the missed packet.

@jamesarich
Copy link
Contributor

It seems like there could be a potential vector for misuse here - if bad actor Bob wanted to, they could use Alice's known node id and randomized public keys to flood Charlie with spoofed direct messages from Alice, causing this DifferentPublicKey interaction to fire over and over.

@GUVWAF
Copy link
Member Author

GUVWAF commented Oct 19, 2025

Yes, we'll definitely need to add some rate limiting for this. Nodes will normally not be generating new keys every day anyway.

@jamesarich
Copy link
Contributor

Yes, we'll definitely need to add some rate limiting for this. Nodes will normally not be generating new keys every day anyway.

Would that rate-limiting responsibility fall to the client? Still potential to jam up a node with ClientNotification spams.

@GUVWAF
Copy link
Member Author

GUVWAF commented Oct 19, 2025

No, we can do that in the firmware.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants