search

agherzan / yubikey-full-disk-encryption

Use YubiKey to unlock a LUKS partition
Apache License 2.0
808 stars 50 forks source link

Use udisksctl and expect to unlock encrypted volume without needing supeuser privileges #17

Closed CristianCantoro closed 6 years ago

CristianCantoro commented 6 years ago

Hi,

first of all, thanks for this nice project, it helped me a lot setting up auto-decrypt with my Yubikey. I am working on Ubuntu (16.04 LTS) so I can say that the scripts work also on this distribution. Also, I was able to get this working with a loop device.

On Ubuntu (and I assume on other distros as well), if you have a removable drive attached on boot, it gets auto-mounted on user login. The mount operation is performed with the user as owner and root privileges (sudo) are not needed. If the volume is encrypted, a (graphical) passphrase prompt is presented to the user.

On the other hand, if you mount the device as root and then you unmount it as a user (say, clicking on the eject icon in nautilus) then you are prompted for your password because the system policy requires administrative privileges to unmount a device mounted by another user, which is understandable, but annoying. The problem is analogous with lock/unlock.

i wanted to mount and unlock the volume as a user, but ykfde-open needed to be executed as root.

Using udisks (udisksctl) it's possible to mount and unlock the volume as a regular (non-root) user. When unlocking, udisksctl prompts the user for the passphrase.

One problem was that there was no obvious way (at least to me) to pass the response of the yubikey to udisksctl's prompt, so I have worked around that using expect. You can see the small expect script embedded in the code.

One caveat is that it is not possible to specify the mapper name for luks, as it is created automatically as /dev/mapper/luks-<uuid-of-the encrypted-volume>. i was using that name anyway, so it did not make any difference to me, but maybe the pull request can be edited so that a warning is presented to the user.

This pull request adds udisks and expect as dependencies. udisks should be present by default in many desktop distros and expect is a "classic" *nix utility that can be easily installed, I expect (pun intended :-) ) it to be available via the various package managers.

Thanks again.

Cristian

Vincent43 commented 6 years ago

I tested your changes and it works thanks!

One nitpick: can you double quote variable at https://github.com/agherzan/yubikey-full-disk-encryption/pull/17/files#diff-ca4ed95aa62e9b234a8b97da5d793c2bR79 ?

It may not matter in this case but it's nice to have consistence.

BTW: There is similar project addressed for ubuntu/debian https://github.com/cornelinux/yubikey-luks You may want to submit your changes there also.

CristianCantoro commented 6 years ago

@Vincent43 said:

One nitpick: can you double quote variable at https://github.com/agherzan/yubikey-full-disk-encryption/pull/17/files#diff-ca4ed95aa62e9b234a8b97da5d793c2bR79 ?

I removed those quotes because I was having an issue, but it's working now. Probably the issue was "coding late in the night" :-)

It may not matter in this case but it's nice to have consistence.

Of course. Fixed, rebased the commit, and pushed.

BTW: There is similar project addressed for ubuntu/debian https://github.com/cornelinux/yubikey-luks You may want to submit your changes there also.

Thanks for the pointer, I will do it.

CristianCantoro commented 6 years ago

(Also, I was thinking that you may want to specify in the readme about the dependency on expect, IIRC it is not installed by default on all systems)

agherzan commented 6 years ago

Looks good to me. Maybe I would remove on spurious new line that I see in the expect script.

I didn't know about that project @Vincent43 . I'm wondering if we could merge the projects. Sounds like duplicated work at this point.

Vincent43 commented 6 years ago

@agherzan the problem is initramfs setup is different in debian. Also cryptsetup is slightly modified, there is /lib/cryptsetup/askpass tool which doesn't exist in Arch. Moreover supporting more than one cryptsetup or other binaries/libs versions can be tricky, someone would have to test that everything works on two platforms.

@CristianCantoro changing https://github.com/agherzan/yubikey-full-disk-encryption/blob/master/PKGBUILD#L8 to: depends=('yubikey-personalization' 'cryptsetup' 'expect' 'udisks2') would be enough for users who build this as package.

agherzan commented 6 years ago

Well I don't think it would be tricky. It would be as it is now. Users will test both flavors but not using separate repos but using a flag or something. If one flavor breaks, it will get fixed. We will be able to reuse tons in this way.

CristianCantoro commented 6 years ago

@agherzan said:

Looks good to me. Maybe I would remove on spurious new line that I see in the expect script.

@Vincent43 said:

@CristianCantoro changing https://github.com/agherzan/yubikey-full-disk-encryption/blob/master/PKGBUILD#L8 to: depends=('yubikey-personalization' 'cryptsetup' 'expect' 'udisks2') would be enough for users who build this as package.

I have removed that extra new line and added the dependencies

@agherzan said:

Well I don't think it would be tricky. It would be as it is now. Users will test both flavors but not using separate repos but using a flag or something. If one flavor breaks, it will get fixed. We will be able to reuse tons in this way.

If I can give my 2 cents, I am a big fan of merging. This project was easier to find for me (probably because I was googling for "yubikey full disk encryption external drive" and similar things) and I have been lucky enough that my use case works with this project. The overlap is for sure significant, at the very least it would be useful if both project mentioned each other in their READMEs.

Maybe we should see what @cornelinux thinks as well.

Vincent43 commented 6 years ago

@agherzan I'm skeptical of merging.

  1. It will make the whole more complicated. This project is using bash and debian ver is using posix. You have to add conditionals - if debian do this if arch do that. Currently when you add new feature you don't have to care if it breaks something on distro you don't use. I doubt that there are many who use both distros simultaneously.

  2. Those projects already share nearly all of the functionality. Configs are different but you can easily generate same passwords with both tools. The only difference is that debian version supports plymouth and arch not (corrected: debian version additionally doesn't support saved challenge mode) . Merging won't provide much benefits for users.

  3. Both projects aren't much active. Someone would have to do the work. After that it may bring benefits but upfront you have to pay that cost. I like those projects and I did everything I can to make them better but I'm really not a programmer (even if we talk about simple scripting) and I don't have much expertise in those matters.

Of course if merge happens I will support and try to help with it.

BTW: We already linked to debian version in readme: https://github.com/agherzan/yubikey-full-disk-encryption#security and that wasn't my work :smile: