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

QR sharing URL format potentially leaks channel keys to remote server #126

Closed
gkelly opened this issue Aug 11, 2020 · 3 comments · Fixed by #127
Closed

QR sharing URL format potentially leaks channel keys to remote server #126

gkelly opened this issue Aug 11, 2020 · 3 comments · Fixed by #127

Comments

@gkelly
Copy link
Contributor

gkelly commented Aug 11, 2020

Right now the URL format for the QR-shared channel details looks like this: https://www.meshtastic.org/c/CBciENTxuzogKQdZ8Lz_q89Oab8qB0RlZmF1bHQ=

Which encodes the channel name, parameters, and key into the URL path. When shared, there's a non-zero chance that this URL gets fetched. Someone taps the link (and admittedly gets a 404) but the URL path is being sent to meshtastic.org. Anyone with access to the hosting logs can potentially see the GET /c/CBciENTxuzogKQdZ8Lz_q89Oab8qB0RlZmF1bHQ= request and take the channel configuration including the key.

I believe there's a fairly easy solution here: placing the channel configuration in the URL fragment instead of the path. The URL fragment identifier is not sent to the server when browsing to the URL. So even if the URL is clicked/tapped/cached by a browser fetch the channel options never get sent to a remote machine.

gkelly added a commit to gkelly/Meshtastic-Android that referenced this issue Aug 11, 2020
Put the channel options into the URL fragment instead of the URL path.
This ensures that the channel options (including the channel key) is
never accidentally leaked to a remote party. This URL format appears to
be backwards compatible, URLs generated in the new form are properly
parsed by the old version. URLs generated by the old version are _not_
parsed by the new version.

Closes meshtastic#126.
@geeksville
Copy link
Member

great find! I totally agree. Though I guess the risk is limited slightly if the person has the meshtastic app already installed, in that case the URL should never get sent off device.

But your fix also sounds great. I'll add a comment there.

@geeksville
Copy link
Member

This issue has been mentioned on Meshtastic. There might be relevant details there:

https://meshtastic.discourse.group/t/alpha-tester-thread-0-9-2-android-app-ready-for-testing/298/237

@geeksville
Copy link
Member

This issue has been mentioned on Meshtastic. There might be relevant details there:

https://meshtastic.discourse.group/t/android-app-new-releases/78/62

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 a pull request may close this issue.

2 participants