ProtonVPN / proton-vpn-gtk-app

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

Review PR? Keyring issue #30

Closed Anonymous941 closed 2 months ago

Anonymous941 commented 4 months ago

Hi,

I've submitted ProtonVPN/python-proton-keyring-linux#1 over a month ago, and was told by @calexandru2018 that it was being discussed internally. Can I have some kind of update on this, even if it's been declined? This has been a very frustrating bug that impacts all my computers constantly, and likely causes issues with others as well. Again, any response would be fine, just please tell me some update.

calexandru2018 commented 4 months ago

Hey @Anonymous941 apologize for the delay, it was really hard these last months.

So going back to the keyring issue. From what we've identified this issue seems to occurs on systems that have KWallet installed by default and auto-login enabled. Due to this (and since it's working on well on Gnome-keyring) we won't be merging this commit since it's working well as is, and that this is mainly quirks caused by KWalletm, also it might have un-intended consequences where it already is working well. We eventually want to work on a dedicated backend for KDE that supports KWallet.

Anonymous941 commented 4 months ago

Hey @Anonymous941 apologize for the delay, it was really hard these last months.

That's okay, thanks for responding! :)

So going back to the keyring issue. From what we've identified this issue seems to occurs on systems that have KWallet installed by default and auto-login enabled. Due to this (and since it's working on well on Gnome-keyring) we won't be merging this commit since it's working well as is, and that this is mainly quirks caused by KWalletm, also it might have un-intended consequences where it already is working well. We eventually want to work on a dedicated backend for KDE that supports KWallet.

This also happens on my computers that use GNOME, and don't have KWallet installed, so it must be another issue unrelated to the PR then...

calexandru2018 commented 4 months ago

@Anonymous941 do you mind confirming if you have auto-login enabled in your gnome setups ?

Anonymous941 commented 4 months ago

@calexandru2018 In the wallet or generally? I do have auto-login enabled for the user on all of my computers, but now one of them randomly doesn't have that issue (and it's running KDE and KWallet) No idea what is different about it and it also has autologin enabled.

Also, Ungoogled Chromium stores things in the keyring, and ProtonVPN keeps deleting that too every time I reboot (there's just a blank keyring), so the issue must be causing that

calexandru2018 commented 4 months ago

KWallet is known not to play well with auto-login, but about gnome keyring I don't recall. Do you mind disabling auto-login in one of your gnome machines and see if the issue persists please ? We're currently working on WG so we might not have enough time to look into this, but we'll give our best.

e34rrsff commented 3 months ago

I got the same issue and have been trying to troubleshoot this. I did have auto-login enabled & am on Ubuntu 22.04 with GNOME.

These are the steps I've tried:

Still, I get the same error "Your session is invalid. Please login to re-authenticate."

Appreciate your efforts

calexandru2018 commented 3 months ago

Hmm that is odd @e34rrsff , do you mind installing that in a VM ? I'd recommend using Boxes which much quicker then any other software out there (on Linux that is). Because our QA has tried to replicate (and also personally me) and we couldn't recreate it.

e34rrsff commented 3 months ago

No luck.

I tried 2 separate 22.04 VM installs; one configured with auto-login from the install & one without, just in case that somehow it affected anything.

Also tried a Fedora VM, since I recall a post mentioning that it worked for them on there, still doesn't work for me.

e34rrsff commented 3 months ago

Really the only other thing I can think of is if somehow physical security keys affect logging in, if you can't reproduce the issue.

calexandru2018 commented 3 months ago

Really the only other thing I can think of is if somehow physical security keys affect logging in, if you can't reproduce the issue.

Do you use physical security keys?

Anonymous941 commented 3 months ago

@calexandru2018 I don't use them, and still have this issue on many computers Also I'll try to replicate this in a VM, @e34rrsff can you share the link to the ISO you're using?

e34rrsff commented 3 months ago

Do you use physical security keys?

I do

@e34rrsff can you share the link to the ISO you're using?

I'm just using the normal ISOs from Ubuntu's and Fedora's websites

Ubuntu: https://releases.ubuntu.com/22.04.4/ubuntu-22.04.4-desktop-amd64.iso Fedora: https://southfront.mm.fcix.net/fedora/linux/releases/39/Workstation/x86_64/iso/Fedora-Workstation-Live-x86_64-39-1.5.iso

e34rrsff commented 3 months ago

@Anonymous941 Well, how about 2FA codes?

Anonymous941 commented 3 months ago

@Anonymous941 Well, how about 2FA codes?

No, just a securely generated password. One thing that I think is important is that it deletes the entire keyring and makes a new one, which removes other programs' keys too. So this should narrow down the bug because it must be with code that deletes the keyring

calexandru2018 commented 3 months ago

@Anonymous941 that is odd and definitely not an issue with our keyring code. If you look at l66 you'll see that we only delete the entry.

Btw do you mind please testing the follwing in the terminal:

While testing this, do try out with both auto-login enabled and disabled. Also please provide me with the output after each command to ensure everything is being properly processed.

Anonymous941 commented 3 months ago

Thank you so much for looking into this issue! I have one computer that runs KWallet, and doesn't have this issue, plus another that also uses KWallet and prompts every time I reboot. Two of them also don't work and use GNOME keyring + autologin.

Here are the results of running those commands on the computer that DOES work

Autologin enabled:

$ keyring --list-backends
keyring.backends.fail.Keyring (priority: 0)
keyring.backends.libsecret.Keyring (priority: 4.8)
keyring.backends.SecretService.Keyring (priority: 5)
keyring.backends.kwallet.DBusKeyring (priority: 5.1)
keyring.backends.chainer.ChainerBackend (priority: 10)
$ keyring set test test
Password for 'test' in 'test': 
$ keyring get test test
test content
$ keyring del test test

Autologin disabled:

$ keyring --list-backends
keyring.backends.chainer.ChainerBackend (priority: 10)
keyring.backends.libsecret.Keyring (priority: 4.8)
keyring.backends.kwallet.DBusKeyring (priority: 5.1)
keyring.backends.SecretService.Keyring (priority: 5)
keyring.backends.fail.Keyring (priority: 0)
$ keyring set test test
Password for 'test' in 'test': 
$ keyring get test test
test content
$ keyring del test test
calexandru2018 commented 3 months ago

Just so it's clear to me, the above tests were run on which distros and with which default DE (assume it's KDE but want to be sure) ?

ghost commented 3 months ago

I have done a bit more testing, happens on:

@Anonymous941 You mentioned one of your computers don't have this issue, if you have distrohopped did that one specific computer continue working fine while all the others did not?

I am thinking there is a small possibility this issue happens on specific hardware because:

High performance processors may avoid this issue by doing something faster related to the keyring? Maybe if everyone posts their cpu we can find a pattern?

Its strange the Fedoras work for me, maybe they have a package which makes the keyring work different?

calexandru2018 commented 3 months ago

@NigoriousT I find it hard to believe that it's related to hardware. If you don't mind, could you test the same things I asked @Anonymous941 to test in my post above please ?

Lintech-1 commented 3 months ago

I use CachyOS and did the following to run protonvpn

sudo vim /usr/share/dbus-1/services/org.freedesktop.secrets.service 
----------------------------------------
[D-BUS Service]
Name=org.freedesktop.secrets
Exec=/usr/bin/kwalletd5
----------------------------------------

but still I have this output and the connections are not reproducible

2024-03-13T11:27:55.628144 | proton.keyring_linux.core.keyring_linux:111 | ERROR | Unexpected keyring "keyring.backends.SecretService.Keyring (priority: 5)" error
Traceback (most recent call last):
  File "/usr/lib/python3.11/site-packages/proton/keyring_linux/core/keyring_linux.py", line 99, in _is_backend_working
    keyring_backend.get_password(
  File "/usr/lib/python3.11/site-packages/keyring/backends/SecretService.py", line 78, in get_password
    collection = self.get_preferred_collection()
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/keyring/backends/SecretService.py", line 61, in get_preferred_collection
    collection = secretstorage.get_default_collection(bus)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/secretstorage/collection.py", line 177, in get_default_collection
    return Collection(connection)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/secretstorage/collection.py", line 45, in __init__
    self._collection.get_property('Label')
  File "/usr/lib/python3.11/site-packages/secretstorage/util.py", line 67, in get_property
    (signature, value), = self.send_and_get_reply(msg)
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/secretstorage/util.py", line 48, in send_and_get_reply
    raise DBusErrorResponse(resp_msg)
jeepney.wrappers.DBusErrorResponse: [org.freedesktop.DBus.Error.NameHasNoOwner] ('Could not activate remote peer: unit failed.',)
2024-03-13T11:27:55.773575 | proton.vpn.connection.vpnconnector:238 | INFO | CONN:STATE_CHANGED | Disconnected (initial state)
2024-03-13T11:27:55.774546 | proton.vpn.app.gtk.app:57 | INFO | APP:PROCESS_START | self=<app.App object at 0x76596217ee40 (proton+vpn+app+gtk+app+App at 0x18b3290)>
2024-03-13T11:27:55.834718 | proton.vpn.app.gtk.services.reconnector.reconnector:96 | INFO | VPN reconnector enabled.
2024-03-13T11:27:57.147895 | proton.vpn.app.gtk.controller:139 | INFO | APP.STARTUP:STARTUP_ACTIONS | Running startup actions
2024-03-13T11:27:57.148074 | proton.vpn.app.gtk.widgets.vpn.vpn_widget:178 | INFO | APP.VPN:WIDGET_READY | VPN widget is ready (load time: 1.32 seconds)
2024-03-13T11:27:57.148163 | proton.vpn.app.gtk.services.refresher.vpn_data_refresher:148 | INFO | APP.VPN_DATA_REFRESHER:ENABLE | VPN data refresher service enabled.
2024-03-13T11:27:57.148235 | proton.vpn.app.gtk.services.refresher.client_config_refresher:68 | INFO | Client config refresher enabled.
2024-03-13T11:27:57.148316 | proton.vpn.app.gtk.services.refresher.client_config_refresher:107 | INFO | Next client config refresh scheduled in 2:37:51.528084
2024-03-13T11:27:57.148368 | proton.vpn.app.gtk.services.refresher.server_list_refresher:74 | INFO | Server list refresher enabled.
2024-03-13T11:27:57.148886 | proton.vpn.session.utils:25 | INFO | API:REQUEST | '/vpn/loads'
2024-03-13T11:27:58.212924 | proton.vpn.app.gtk.services.reconnector.reconnector:176 | INFO | Network connectivity was detected.
Anonymous941 commented 3 months ago

You mentioned one of your computers don't have this issue, if you have distrohopped did that one specific computer continue working fine while all the others did not?

It's a ThinkPad T470s, and what's strange it that it had the issue for months, then one day it just permanently stopped asking me for my password. I'm using KUbuntu and haven't changed distros on that computer but have upgraded to 22.04 (I think it was before that issue appeared). I have no idea why

Anonymous941 commented 3 months ago

Just so it's clear to me, the above tests were run on which distros and with which default DE (assume it's KDE but want to be sure) ?

@calexandru2018 KUbuntu, and I can run it on other computers that do have that issue if you want

to test in my post above please ?

Which post? I posted the output of the keyring commands if that's what you meant

I also have another computer with me now so I can run the commands with autologin enabled. This is running Ubuntu, GNOME, GNOME wallet and has the issue.

$ keyring --list-backends
keyring.backends.chainer.ChainerBackend (priority: 10)
keyring.backends.kwallet.DBusKeyring (priority: 4.9)
keyring.backends.libsecret.Keyring (priority: 4.8)
keyring.backends.SecretService.Keyring (priority: 5)
keyring.backends.fail.Keyring (priority: 0)
$ keyring set test test
Password for 'test' in 'test': 
$ keyring get test test
test content
$ keyring del test test
calexandru2018 commented 3 months ago

This is running Ubuntu, GNOME, GNOME wallet and has the issue.

So the one that has this keyring issue, didn't display any errors with the keyring commands above ? If so could you try setting an entry, reboot and then try to get it again to ensure it's being persisted to keyring please ?

ghost commented 3 months ago

@calexandru2018 Ubuntu 22.04, same result with auto login off and on:

d@d:~/Desktop$ keyring --list-backends keyring.backends.fail.Keyring (priority: 0) keyring.backends.libsecret.Keyring (priority: 4.8) keyring.backends.SecretService.Keyring (priority: 5) keyring.backends.chainer.ChainerBackend (priority: 10) d@d:~/Desktop$ keyring set test test Password for 'test' in 'test': d@d:~/Desktop$ keyring get test test d d@d:~/Desktop$ keyring del test test d@d:~/Desktop$

Edit: I don't know why its crossing this out above, maybe some github formatting I don't understand?

On Fedora Workstation 39, the only difference is this command returns the strings in a different order:

$ keyring --list-backends keyring.backends.libsecret.Keyring (priority: 4.8) keyring.backends.chainer.ChainerBackend (priority: 10) keyring.backends.fail.Keyring (priority: 0) keyring.backends.SecretService.Keyring (priority: 5)

calexandru2018 commented 3 months ago

@NigoriousT could you please also test the following

could you try setting an entry, reboot and then try to get it again to ensure it's being persisted to keyring please ?

ghost commented 3 months ago

@calexandru2018 Ubuntu 22.04

$ keyring set test test (enter "d") $ reboot $ keyring get test test

brought up the same pop up as if I had opened protonvpn, and did not remember the key I set.

calexandru2018 commented 3 months ago

@NigoriousT the password you set was "d" in this case. But ok that at the least is some progress. It means that the keyring backend package that we rely on is not storing the data. Can you confirm me the following please:

I'll try to replicate this again on my side to see what happens with the info you provided and comeback to you.

ghost commented 3 months ago

@calexandru2018 Its a clean Ubuntu 22.04 in Gnomeboxes with its default pre-customized gnome. I did not install literally anything except ProtonVPN as instructed by your website. I have tested both auto login settings once again and it happens on both.

calexandru2018 commented 3 months ago

@NigoriousT that is really odd. I just recently installed the latest 22.04.04 and tried it out in Gnome Boxes, this is what I did:

Edit: Even logged in into the client, rebooted and upon launching the client I was prompted again for the keyring password, which I typed in and the client was in logged in state.

Screenshot from 2024-03-19 08-44-47 Screenshot from 2024-03-19 08-45-42 Screenshot from 2024-03-19 08-45-52

ghost commented 3 months ago

@calexandru2018 I think it's caused by the keyring password being blank. I usually install Ubuntu with auto login turned on in the graphical installer which im pretty sure causes the keyring password to be blank. I set the password to something not null and now it behaves as you describe right above.

calexandru2018 commented 3 months ago

Thanks for the feedback @NigoriousT , this has really helped us to narrow down the issue, and we found out what the possible issue is, and why the PR actually solves it (in certain limited scenarios like this).

Now that we have a better understanding of how things works and what is happening, we'll be merging this PR as it fixes this edge case (and honestly it was hard to predict it). Once the packages with this commit is published we'll close this ticket.

NOTE: Be aware thought that if you have auto-login set, you'll always be prompted to unlock the keyring whenever you want to open the app for the first time after a reboot/logout or something similar. This is expected behavior from the keyring side because if you auto-login and have a password set you'll need to type your keyring password to unlock it.

NOTE2: After this fix, if you restart your machine you'll be logged out and prompted to set the password for they keyring because our app is requesting it. I've tested this and unfortunately this means that it won't be possible to have a no-password keyring.

Anonymous941 commented 3 months ago

I can't tell you how happy I am now that the issue has finally been found! Thank you for fixing it, the Linux client is now finally usable! Also, I want to clarify that @Wizzerinus is the one who actually found the bug, and I just made the PR

After this fix, if you restart your machine you'll be logged out and prompted to set the password for they keyring because our app is requesting it. I've tested this and unfortunately this means that it won't be possible to have a no-password keyring.

How do other apps implement it? This might be a problem for my computers that don't have a password at all, because I can't automatically unlock it with the login password. I might be able to set some dummy password on the user and enable auto-login, but that seems to cause issues as noted above...

calexandru2018 commented 3 months ago

I can't tell you how happy I am now that the issue has finally been found! Thank you for fixing it, the Linux client is now finally usable! Also, I want to clarify that @Wizzerinus is the one who actually found the bug, and I just made the PR

Gotcha thank you for mentioning it. The problem seems to be actually when the keyring stores (without password) and for some reason it's not storing in the json format that we're setting, so not sure if it's a bug of an underlying package that we use or just a bug of gnome keyring. But now at the least we've managed to reproduce.

After this fix, if you restart your machine you'll be logged out and prompted to set the password for they keyring because our app is requesting it. I've tested this and unfortunately this means that it won't be possible to have a no-password keyring.

How do other apps implement it? This might be a problem for my computers that don't have a password at all, because I can't automatically unlock it with the login password. I might be able to set some dummy password on the user and enable auto-login, but that seems to cause issues as noted above...

Other apps might be using sql databases, but this is just the default behavior of gnome keyring (kwallet does not even work with that feature IIRC)

calexandru2018 commented 3 months ago

So we've investigated this further and created a discussion with gnome to see if this could be fixed at gnome-keyring level or not, feel free to follow it: https://discourse.gnome.org/t/possible-bug-or-feature-storing-getting-data-keyring-protected-vs-unprotected-keyring/20312

calexandru2018 commented 2 months ago

So given this is a gnome-keyring bug, we'll release the fix to beta this week, though bear in mind, for the person that made the MR we won't be able to merge your MR locally into gitlab because the fix was done in the https://github.com/ProtonVPN/python-proton-keyring-linux-secretservice/ mainly because the issue is with gnome-keyring. We could at the least mention your name if that is ok with you in the commit @Wizzerinus

Wizzerinus commented 2 months ago

Yeah sounds good o7

calexandru2018 commented 2 months ago

These two packages should solve the issue, please give them a try. If all goes well all updates go to stable next week.

Edit: Either download them from here and install it manually or just switch to the beta branch and install the app that way, as it contains more fixes that should be released next week.

Edit2: if you're on F38 just replace 39 by 38 in the URL.

calexandru2018 commented 2 months ago

The fixes are public:

Feel free to close the ticket please, if not closed within ~30days I'll close it myself assuming that it was solved for everybody :)

I'll be closing the MR, thank you all 👍🏻