Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request]: Don't uplink decrypted precise location to MQTT #5404

Open
juliatuttle opened this issue Nov 20, 2024 · 9 comments
Open
Assignees
Labels
enhancement New feature or request

Comments

@juliatuttle
Copy link

juliatuttle commented Nov 20, 2024

Platform

Cross-Platform

Description

As part of tightening handling of precise location as PII, when uplinking a position packet to MQTT:

  • If it's decrypted (protobuf or JSON), blur the location to imprecise before uplinking.
  • If it's encrypted with a non-default key, consider it secured in transit between users of that key, and uplink it intact.
  • If it's encrypted with a default key, it's not effectively secured, but it can't be blurred as-is, so you'd need to either drop it entirely, decrypt/blur/uplink it decrypted, or decrypt/blur/encrypt/uplink it encrypted. The last is the most complex, but would cause the least change to the current behavior.

It's worth considering whether this behavior should differ when uplinking to private brokers; my instinct is that it should apply to private brokers as well, since part of the concern that inspired this bug was that data can be relayed between (some) brokers.

@juliatuttle juliatuttle added the enhancement New feature or request label Nov 20, 2024
@thebentern
Copy link
Contributor

thebentern commented Nov 20, 2024

We already do this on the official broker. I think the better approach here is to hard enforce the firmware not uplinking of MQTT packets without the OkToMQTT bit flag, under explicit consent. This will both give users the choice to opt-in and the freedom to continue to uplink precise data to private brokers as desired. Removing this functionality entirely would be a very controversial change.

@juliatuttle
Copy link
Author

juliatuttle commented Nov 21, 2024

My understanding from chatting with Garth was that relying on "OK to MQTT" as consent for the app/node to transmit precise location on default keys and for any other user to relay that precise location to MQTT was insufficient, for two reasons:

  1. Practically, users are apparently "shocked" to see their precise location visible to anyone on the Internet and collected by third-party maps. This isn't surprising, as users have to build a mental model of how the GPS, channel, precision, and MQTT settings interact before they can understand the impact of their choices!
  2. Legally, Meshtastic is responsible for safeguarding the PII they collect in their app, and for ensuring anyone they send that data to will safeguard it in turn. Part of that safeguarding is allowing users to delete collected Pii, which is entirely infeasible when it's visible in plaintext to anyone nearby. This is unfortunate given Meshtastic's open nature, but to many users, it's a discrete product that should follow their privacy rights/expectations.

@thebentern
Copy link
Contributor

thebentern commented Nov 21, 2024

Legally, Meshtastic is responsible for safeguarding the PII they collect in their app, and for ensuring anyone they send that data to will safeguard it in turn.

We do not collect any PII. Any parties doing so are responsible for this data, not Meshtastic. Furthermore, implementing your change as described, given the open-source nature of the project, someone only needs to comment out a few lines of code restoring the uplink of this data, and based on the popularity of the feature, I fully expect this to occur. All of these efforts to try to prevent inadvertent uplink need a line drawn in the sand somewhere. My opinion is throwing the whole feature out is unacceptable. The firmware level enforcement of this relies entirely on good faith acting by uplink nodes.

Practically, users are apparently "shocked" to see their precise location visible to anyone on the Internet and collected by third-party maps. This isn't surprising, as users have to build a mental model of how the GPS, channel, precision, and MQTT settings interact before they can understand the impact of their choices!

While I agree about the uplink of this information to 3rd party brokers being a surprise (which we will close with this flag enforcement), you have to explicitly turn on precise position. The default configuration does not lead you down this path, so it shouldn't entirely be a surprise if you know that you're sending locations with full precision, and you know that the ecosystem of MQTT uplink exists. Perhaps the apps have more explicit warnings here about the information being broadcasted, but that is not the responsibility of the firmware.

@juliatuttle
Copy link
Author

Legally, Meshtastic is responsible for safeguarding the PII they collect in their app, and for ensuring anyone they send that data to will safeguard it in turn.

We do not collect any PII. Any parties doing so are responsible for this data, not Meshtastic.

This is very much not what Garth seemed to believe in Discord yesterday, but I don't know who's right.

Furthermore, implementing your change as described, given the open-source nature of the project, someone only needs to comment out a few lines of code restoring the uplink of this data, and based on the popularity of the feature, I fully expect this to occur.

This wouldn't be the only line of defense; it would work alongside changes to the client apps that Garth already made in iOS and was seemingly planning for Android.

All of these efforts to try to prevent inadvertent uplink need a line drawn in the sand somewhere. My opinion is throwing the whole feature out is unacceptable.

To be clear, this isn't throwing the whole feature out; it's specifically removing just enough data that the location isn't considered sensitive data.

While I agree about the uplink of this information to 3rd party brokers being a surprise (which we will close with this flag enforcement), ...

To be fair, that flag enforcement code could be removed just as easily as my suggested location blurring code.

...you have to explicitly turn on precise position.

True, but...

...and you know that the ecosystem of MQTT uplink exists.

...it's entirely possible for someone to not know about the ecosystem of MQTT uplink.

Fundamentally, I think the ethical requirement here is to make it very clear to users when they're about to share their location publicly, that anyone at all can see it, whether it's precise or imprecise, whether they're granting permission for others to uplink it, and what that all means.

Right now, a sufficiently motivated and technical user can figure this all out, but there's not a single, clear, meaningful choice for users who just want to get things working and/or aren't as technical.

I'd be happy to try to write some copy explaining the impact of a user's settings clearly, whether or not precise location on default keys sticks around.

@seagull9000
Copy link

seagull9000 commented Dec 14, 2024

I don't think it is really clear to users how this works currently.

For example, it appears that if the MQTT node has both default channel and private channel, if the precise position is received on the private one, the precise position is uploaded to the map because it was successfully decrypted by the MQTT node. I don't think this is what the user intended or expected, but there is little explanation that this is what will happen.

Caveat - at least I think that is what happend. Again, it is not clear.

BTW It's a great project with lots of effort going into it. Thanks all.

@seagull9000
Copy link

I might add, that the private channel was set to not upload on the sending device and the mqtt node. That is why the user was a bit surpised to see their precise location on the map.

@mntn-xyz
Copy link

mntn-xyz commented Dec 20, 2024

I think that a loud and explicit warning (possibly with checkbox nag screen) when sharing location with MQTT would be better than completely eliminating the ability to share precise location if desired.

For example, it appears that if the MQTT node has both default channel and private channel, if the precise position is received on the private one, the precise position is uploaded to the map because it was successfully decrypted by the MQTT node. I don't think this is what the user intended or expected, but there is little explanation that this is what will happen.

This is indeed confusing and unexpected behavior. (Edit: may not be accurate, see comment below. If this is not what happened then IMO that further reinforces the need for stronger warnings re: location sharing during configuration.)

@GUVWAF
Copy link
Member

GUVWAF commented Dec 20, 2024

For example, it appears that if the MQTT node has both default channel and private channel, if the precise position is received on the private one, the precise position is uploaded to the map because it was successfully decrypted by the MQTT node. I don't think this is what the user intended or expected, but there is little explanation that this is what will happen.

This is not intended behaviour and I also don't see how this could ever happen in the firmware. MQTT checks specifically on which channel the packet was received, and only if uplink is enabled on that channel, it will publish it. And even then, mqtt.encryption_enabled had to be set to false, otherwise the MQTT server can't decrypt it. And now with 2.5, the sending node even has to set "OK to MQTT" to true.

Are you sure it wasn't already uploaded when the precise location was shared (once) on the public channel?

@seagull9000
Copy link

Are you sure it wasn't already uploaded when the precise location was shared (once) on the public channel?

That is entirely possible. I'll see if I can replicate when this happens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants