initial yubikey piv support

[tomis007]

Hey! Super cool project. I wanted to add yubikey support with its piv feature. Here's my initial PR for that. It's a work in progress -- please do not merge the code is still hacky, but I made the PR to get your take on my implementation so far. A few notes:

• the go-piv/piv-go project has a bug where it doesn't actually export a few of the key types because they have lowercase names. I submitted export KeyRSA and KeyEd25519 go-piv/piv-go#164 to fix that
• I'm storing priv in the config passed to the yubikey type but I am not sure if that's needed or if it's better to just prompt every time instead
• I put one private key in the 9C slot (https://docs.yubico.com/yesdk/users-manual/application-piv/slots.html) and used that for all 3 of the required keys that are generated for secure boot. Is there a better way to do this? Maybe 3 different ones in the other slots? My current implementation works as it requires a pin for every signature with the key https://developers.yubico.com/PIV/Introduction/Certificate_slots.html
• I sort of had to hack it together because it doesn't look like the config file is finished yet. I could fix that up too to make it easier to use the yubikey piv signing
• I'm not storing the private key on disk, instead storing the public key and using that to request the private key from the yubikey when needed. I think that makes the most sense

Thanks!

[Foxboron]

This can probably be left as a TODO until I figure out the PIN/password support.

[tomis007]

I think here I'll want it to prompt for the Yubikey PIV pin when it's not 123456. Were you talking about a different PIN?

[Foxboron]

I'm thinking generally how to deal with password prompts in sbctl. It's not a thing yet and that needs to be planned a bit.

[tomis007]

I ended up using https://github.com/manifoldco/promptui in my yubikey backend file, which works well and fits the general aesthetic / workflow of sbctl

[Foxboron]

Thanks for working on this!

the go-piv/piv-go project has a bug where it doesn't actually export a few of the key types because they have lowercase names. I submitted go-piv/piv-go#164 to fix that

I agree with upstream and you should just use the crypto.Signer interface.

I'm storing priv in the config passed to the yubikey type but I am not sure if that's needed or if it's better to just prompt every time instead

Prompt everytime I reckon. We want to be able to have a different backend pr type so it makes sense to not pass this around.

I put one private key in the 9C slot (https://docs.yubico.com/yesdk/users-manual/application-piv/slots.html) and used that for all 3 of the required keys that are generated for secure boot. Is there a better way to do this? Maybe 3 different ones in the other slots? My current implementation works as it requires a pin for every signature with the key https://developers.yubico.com/PIV/Introduction/Certificate_slots.html

Preferably one key pr key on the yubikey. This reflects the proper key hierarchy and allows us flexibility.

I sort of had to hack it together because it doesn't look like the config file is finished yet. I could fix that up too to make it easier to use the yubikey piv signing

Preferably work on the config parts of it, and if anything is too difficult or problematic please just tell me and I'll write up the code you need.

I'm not storing the private key on disk, instead storing the public key and using that to request the private key from the yubikey when needed. I think that makes the most sense

I wonder if we should store some extra metadata around the key. So we can properly look it up when needed. What happens if we have multiple key?

[tomis007]

Thanks for the review! I'll keep working on it. I am thinking of using a .toml config file instead of .yaml, do you have any strong preference? I personally don't really like using .yaml as a config format because of how complicated it can be

[Foxboron]

I'd prefer if we stick with sbctl.conf. there isn't a need to invent something new only for yubikey. If you need to serialize some state you can just use json

[tomis007]

I'd prefer if we stick with sbctl.conf. there isn't a need to invent something new only for yubikey. If you need to serialize some state you can just use json

Will do! I'll keep working on the PR

[tomis007]

I've been testing out my PR branch with the following sbctl.conf and the sbctl create-keys --type yubikey command. I noticed there's no one option in the .conf to set a directory for everything so I added that as datadir. Also the config parsing code currently does not attempt to make directories. I think the PR is ready for another review / to see what its missing. I also need to update the dependencies in the readme for the yubikey packages. I updated it to store .json data for the yubikey information. Let me know what other changes you think it needs

datadir: /var/lib/sbctl/yubikeys

db_additions:
  - microsoft
files:
  - path: /boot/vmlinuz-linux-lts
  - path: /usr/lib/fwupd/efi/fwupdx64.efi
    output: /usr/lib/fwupd/efi/fwupdx64.efi.signed
keys:
  pk:
    type: yubikey
  kek:
    type: yubikey
  db:
    type: yubikey

[Foxboron]

I'm busy at congress and stuff until new years. So I can't promise I'll get to this until over new years :)

[tomis007]

no worries! have a good new years!

[tomis007]

I did a pacman -Syu today and noticed something with the post hooks for mkinitcpio and pacman. According to https://bbs.archlinux.org/viewtopic.php?id=255944, pacman explicitly does not allow prompting for user input during a post script and all the scripts aren't run in ttys. Unsure exactly what the best way to ask for the yubikey pin in this case is as no matter what stdin is set to ^D / EOF. It might be easiest to add a message to sbctl yubikey signing that if it's not running in a tty to print without doing anything and output a message telling you to rerun the command yourself, let me know what you think

[peto59]

Might I suggest systemd-ask-password? It does have --no-tty flag and with --timeout=0 it will wait indefinitely. I use it to query passwords during boot which has similar constraints.

[tomis007]

Oh that’s cool, it looks like it might work. I will test it out, thanks!

[tomis007]

I fixed the non tty issue with systemd-ask-password

[tomis007]

I think it's ready for another review, I have it working locally on my machine for signing my uki and with the hooks, but further testing from someone else would be good

[Foxboron]

Would it be possible for you to work a bit more on the commit history of this PR? Having 13 commits with non-descriptive names makes it harder to review properly.

[tomis007]

Sure, let me know if that helps. It's probably best to focus on the final commit most. I didn't bother squashing the commits for the pr