search

ProtonVPN / proton-vpn-gtk-app

Official ProtonVPN Linux app
https://protonvpn.com/download-linux
GNU General Public License v3.0
215 stars 28 forks source link

Finally found the keyring problem #16

Closed Anonymous941 closed 6 months ago

Anonymous941 commented 10 months ago

Many keyrings These are either blank, contain just the ProtonVPN login password, or contain that password and some VPN configurations. This is on GNOME, so kde-wallet cannot be causing the issue. Could it be caused because I also have keyrings named Default keyring? It seems very likely that the issue is that it fails to detect the keyring named Default, and makes another one every single time the app is restarted. This means you have to login after every reboot This is happening on all of my computers, some with KDE and some with GNOME, and probably caused #8

Anonymous941 commented 10 months ago

Found something else: it makes the login proton-sso-accounts permanently, and while running, the login proton-sso-account-xxxxx:

Keyring screenshot

The problem is the proton-sso-account-xxxxx is deleted every time the app is restarted for some reason (after starting it again), causing you to have to re-login each time. I'm not sure why it makes multiple keyrings, however

Wizzerinus commented 9 months ago

For anyone wondering, I found the possible cause of this bug. Note that this may be setup dependent and not fix the bug for every single user of this app. (This was also reported through the support system)

So, ProtonVPN includes a number of PEM-encoded certificates in their secret files. They look like this:

-----BEGIN PUBLIC KEY-----
SomeLongBase64String
MaybeAnotherOneHere
-----END PUBLIC KEY-----

As you can see, there are newline breaks within this certificate. So, when ProtonVPN saves this certificate into the keyring, it gets saved as is. Note that ProtonVPN uses json files to store their secrets, so they're actually saving something like this:

{"ServerPublicKey": "-----BEGIN PUBLIC KEY-----\nSomeLongBase64String\nMaybeAnotherOneHere\n-----END PUBLIC KEY-----\n"}

When the app reads the secrets however, the \n sequences are magically transformed into \n characters, so the variable looks like this now:

{"ServerPublicKey": "-----BEGIN PUBLIC KEY-----
SomeLongBase64String
MaybeAnotherOneHere
-----END PUBLIC KEY-----
"}

As you can see this is not a valid JSON file, the github's syntax highlighter also agrees. So, this cannot be parsed by the code, and then the following happens:

https://github.com/ProtonVPN/python-proton-keyring-linux/blob/5ff3c7f9a1a162836649502dd23c2fbe1f487d73/proton/keyring_linux/core/keyring_linux.py#L63

This code deletes the secret whenever parsing fails, creating this bug.

You can bugfix this yourself by editing your copy of keyring_linux.py to include something like stored_data = stored_data.replace("\n", "\\n") before parsing the data with JSON. (Again, not sure if this is the cause of the bug for everyone, because it depends on a lot of factors! Sorry if this does not work for you, but it did work for me which is why I'm sharing it.)

Anonymous941 commented 9 months ago

@Wizzerinus Thank you, this worked and I finally have a fix for this very annoying and persistent bug!

I've submitted a PR, and in the meantime it can manually be patched by copying my fork's keyring_linux.py to /usr/lib/python3/dist-packages/proton/keyring_linux/core/keyring_linux.py

Anonymous941 commented 9 months ago

I will leave this issue open until the PR is merged upstream, @calexandru2018 can you take a look at it?

calexandru2018 commented 9 months ago

Hey @Anonymous941 thanks for the PR (and thank you @Wizzerinus for taking your time in identifying and fixing the issue). I'll take this PR and discuss this internally with our team.

Anonymous941 commented 8 months ago

Hey Anonymous941 thanks for the PR (and thank you Wizzerinus for taking your time in identifying and fixing the issue). I'll take this PR and discuss this internally with our team.

@calexandru2018 Mind taking a look at this again? I'd appreciate it :)

Anonymous941 commented 8 months ago

Hey Anonymous941 thanks for the PR (and thank you Wizzerinus for taking your time in identifying and fixing the issue). I'll take this PR and discuss this internally with our team.

@calexandru2018 Any updates one month later?

calexandru2018 commented 7 months ago

Hey all, could you please send issue reports through the app so we have some logs on this please ? I talked internally to our QA and we're a bit unsure why this is happening with gnome backend (can't reproduce on our side on gnome backends), it is expected with KDE as kwallet is a bit more tricky. Please refer to this ticket in the submission: https://github.com/ProtonVPN/proton-vpn-gtk-app/issues/35

calexandru2018 commented 6 months ago

Closing this and any further discussions should be on https://github.com/ProtonVPN/proton-vpn-gtk-app/issues/30