Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to manage keyslots using Yubikey #76

Closed
wants to merge 14 commits into from

Conversation

crispy-landslide
Copy link

As pointed out in #74, there was no way to manage keyslots using a Yubikey protected passphrase.

I added three additional scripts.

  1. yubikey-luks-add - Adds the ability to add a new Yubikey using an existing Yubikey
  2. yubikey-luks-add-password - Adds the ability to add a passphrase protected keyslot using an existing Yubikey
  3. yubikey-luks-remove - Adds the ability to remove either a passphrase or a Yubikey keyslot using a Yubikey

@Vincent43
Copy link
Contributor

Vincent43 commented Dec 15, 2021

I think it would be much simpler to add new options to existing scripts rather than whole new scripts where you have to copy-paste most of the code which needs to be synced among many files.

@crispy-landslide
Copy link
Author

Totally agree, that would be better. I can try to work on getting a working implementation.
I can add the options to the yubikey-luks-enroll script to be able to use an existing Yubikey, but there isn't a dedicated script for just clearing a keyslot. There is a CLEAR_SLOT option in the yubikey-luks-enroll script that I could modify, but should there be a standalone script?

@crispy-landslide
Copy link
Author

After updating the scripts, we're left with a total of three scripts that can work with keyslots.

  • yubikey-luks-enroll now has the option of using a Yubikey passphrase to add new keyslots
  • yubikey-luks-remove adds the ability to remove keyslots using a Yubikey passphrase
  • yubikey-luks-open remains unchanged

@Vincent43
Copy link
Contributor

Vincent43 commented Dec 16, 2021

There is a CLEAR_SLOT option in the yubikey-luks-enroll script that I could modify, but should there be a standalone script?

I don't see the need for new script when the same functionality already exist.

@crispy-landslide
Copy link
Author

The issue is that the CLEAR_SLOT option does not allow a user to clear a keyslot if the existing passphrase they're using is protected by a Yubikey. I could move most of the contents from the standalone script into the CLEAR_SLOT option to allow that functionality, but it would make more sense to me to have it as its own script and just call that script from the CLEAR_SLOT option.

Another reason for having a standalone script for removing a keyslot is that the user may just want to remove a keyslot without replacing it with another passphrase. Right now, if a user wants to clear a keyslot without replacing it, they can use the CLEAR_SLOT option of the yubikey-luks-enroll script and then just accept the error they get when they don't actually replace it. This workflow seems a bit wonky to me.

Finally, if the goal of this project is to allow the use of Yubikeys for working with LUKS encrypted disks, then wouldn't it make sense to mirror the LUKS extensions for cryptsetup?

  • luksAddKey --> yubikey-luks-enroll
  • luksOpen --> yubikey-luks-open
  • luksSuspend --> yubikey-luks-suspend
  • luksRemoveKey --> no equivalent
  • luksFormat --> no equivalent

@crispy-landslide
Copy link
Author

crispy-landslide commented Dec 16, 2021

@Vincent43 After taking a look through the Arch implementation at https://github.com/agherzan/yubikey-full-disk-encryption, I see that they do not have a separate script for removing a key, and I like their implementation for using an old yubikey passphrase.

If you think it would be a better fit, I can try porting their implementation over to this project instead of the implementation I suggested earlier.

EDIT:
At that point would it be better to just try to add Debian/Ubuntu support to the Arch version? If so, I can close this merge request and try helping out with that effort.

@Vincent43
Copy link
Contributor

Yes, debian/ubuntu support for aforementioned project would be much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants