minecraft-linux / mcpelauncher-manifest

The main repository for the Linux and Mac OS Bedrock edition Minecraft launcher.
https://minecraft-linux.github.io
GNU General Public License v3.0
925 stars 100 forks source link

Use the OS keyring to encrypt tokens #1024

Open lucasmz-dev opened 1 month ago

lucasmz-dev commented 1 month ago

Is your feature request related to a problem? Please describe. Users that don't use a password don't seem to have proper security of their tokens.

Describe the solution you'd like The OS keyring could be a useful way to solve this, and maybe even improve the encryption for the people who do use the passwords. It's a way to encrypt data at rest without needing users to input another password or give another app a password they might use in multiple places.

Describe alternatives you've considered None really, this is how most apps deal with this.

Additional context Noticed the encryption feature for the login, and it might've not even been implemented at that point if was a thing.

ChristopherHX commented 1 month ago

Discussed in discord, https://discord.com/channels/429580677617418240/452451848066957314/1267278022348902460 this is techncally feasible to do.

But I don't think it makes sense to put all the encrypted entries to the keyring, we use aes512 so a key in keyring would be enough? What do you think.

lucasmz-dev commented 1 month ago

Oh wow AES512 is... interesting... People usually use 256, in TLS it's usually 128.

You can do a lot with just one key, you can use it to encrypt other keys, which would be enough, and I think that's what a lot of apps do, I only see one entry for Signal for example.

I'd join the discord to check what was discussed but I don't have discord...

ChristopherHX commented 1 month ago

I amend my comment aes-256 is used

ChristopherHX commented 1 month ago

This can be found on discord, quoted below

GameParrot (contributor)

also could we use https://github.com/frankosterfeld/qtkeychain for securely storing google login info

Me

You can try to implement this using this tmp branch https://github.com/minecraft-linux/mcpelauncher-ui-manifest/tree/qt-keychain Dependencies should work like that, we get a new external dependency that was new in ub 16.04 so you are lucky. I don't think the google credential itself should be stored there (that are a bunch of entries, and aes isn't weak if the key is secured), but the key could be. Imagine you have multiple installs of mcpelauncher of a user (via XDG env), so every instance need unique keys maybe a guid stored in plain text Link chain https://github.com/minecraft-linux/mcpelauncher-manifest/issues/1024

lucasmz-dev commented 1 month ago

What is currently encrypted with the AES256 key? as long as it isn't stored on plaintext when a secret service is available and used, then you can bundle as much as you want with it.

lucasmz-dev commented 1 month ago

It could even be a more secure option, as more things can be encrypted at rest without as much development overhead.

ChristopherHX commented 1 month ago

What is currently encrypted with the AES256 key?

Those are always encrypted even if you don't set a password. (as long you don't used an older version for login) Without custom password, one is generated however as of now stored in plain text (no security, only breaks extraction tools that has not been updated)

A benefit for a keyvault for me would be biometric authentication with an ok button

On the other hand my current linux setup requires password in any case so there is no win for password encryption for me in short term.

I don't know if the flatpak can access the keyring via to it's sandbox

lucasmz-dev commented 1 month ago

Usually flatpaks can use the keyring if they desire they just need an extra perm I usually notice session bus access to org.freedesktop.Secrets in apps that use the keyring