shavee icon indicating copy to clipboard operation
shavee copied to clipboard

Ignore already loaded key

Open edillmann opened this issue 2 years ago • 2 comments

Hi,

It would by nice to test if key is already load before asking for a password.

Currently it give an error Error in mounting user ZFS dataset: Key load error: Key already loaded for

Thanks,

edillmann avatar Mar 30 '22 12:03 edillmann

Thanks for reporting this issue. Would you mind to help more by answering these questions?

  1. Is it correct to assume that the previous loaded key is the "correct one"? Or does shavee need to unload the previous key and load the one that it generated?
  2. Which version of shavee are you using? PR #19 (will up tick the version to 0.5) is still waiting to be merged, and I am wondering if you get the same error if you try v0.5?

kiavash-at-work avatar Mar 30 '22 16:03 kiavash-at-work

I put some extra thoughts into this and believe that the current behavior is consistent with ZFS tools and there is no need to change it.

One potential improvement is to exit with the same error codes as ZFS tool returns (not just a generic number). That however will require a major re-write of error reporting.

kiavash-at-work avatar May 06 '22 22:05 kiavash-at-work

I have been experimenting with and testing shavee for over 30 days with Yubikey configuration. This issue open by @edillmann still needs a lot of attention IMO.

  • Imagine a multi-user system, where user-A logs into the system and PAM loads shavee, and the user is logged in with success.

  • Then sometime later user-A logs out. shavee makes no attempt to lock the ZFS dataset where $HOME is configured. (truthfully this is very similar to ecryptfs behavior on many distros.)

  • Now user-A attempts to login to a graphical or console session again, but they get a failure message. Their ZFS dataset key is already loaded. Thus they are unable to login a 2nd, N, time.

  • Manual intervention by root user, or potentially some non-shavee user with sudo permissions is required to unload their dataset key for user-A to log back in with PAM shavee integration.

  • Only other option besides admin user is a full Reboot, to force the zfs dataset key unload, and then shavee reload zfs dataset key, and let the user log in. A full reboot seems like a nuclear option, but I have found myself doing it.

The main use case where I see this being a huge problem is in servers where high-uptime requirements are mandatory. Perhaps the manual intervention by an admin user is acceptable for some, but a full-reboot will be a non-starter for most situations.

I know that the primary use-case is desktops and laptops, but it would be great if shavee was more useable for servers too. Thanks for this wonderful project and thanks for consideration!

cmosetick avatar Feb 16 '23 17:02 cmosetick

Thank you @cmosetick for testing the Yubikey and PAM module where, the unit and integration tests are still in the TODO list #21, and feedback/help is highly appreciated.

In your described scenario, it seems that this step has the problem statement:

Then sometime later user-A logs out. shavee makes no attempt to lock the ZFS dataset where $HOME is configured. (truthfully this is very similar to ecryptfs behavior on many distros.)

I looked into the PAM module close_session and the code added in patch #17 is supposed to do what was in step 2, unmount and then unload the ZFS key upon logout.

Can you help by providing some error logs (if there are any) pin-pointing to the issue?

kiavash-at-work avatar Feb 16 '23 19:02 kiavash-at-work

Hi @kiavash-at-work I'm afraid that some of the behavior I have seen might be distro specific and not totally because of shavee PAM module.

For what it is worth, I have been testing shavee primarily on a unprivileged user accounts to help with testing.

I will see what possible helpful logs I can find for you regarding user session logout with regard to the PAM module.

cmosetick avatar Feb 16 '23 20:02 cmosetick

Shavee should now with the latest commit ignore already loaded and mounted datasets.

ashuio avatar Nov 18 '23 14:11 ashuio

Hi @cmosetick I know it's been a while, but your mentioned use case should completely supported now like you mentioned.

ashuio avatar Nov 18 '23 16:11 ashuio