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

Allow generating the private key on the computer #72

Closed
wants to merge 1 commit into from

Conversation

jstasiak
Copy link

@jstasiak jstasiak commented Jan 6, 2021

This is to allow creating backups of the private key in case the YubiKey
gets lost or damaged. The word "insecurely" is appended to the name of
the switch and a long warning produced in order to make people who don't
know what they're doing turn away and stop what they're doing.

This is possible since piv-go 1.7.0[1] so I allowed myself to bump the
version of this requirement.

While I do have several doubts about approaching this (see below) I believe this
is a useful thing to have. What I'm unsure about is:

  • The code quality (I'm not writing Go day to day)
  • I may have done something stupid cryptography-wise here, I'm not a cryptographer or a security specialist
  • How to warn properly so that people not knowing what they're doing don't use the new switch – I think I've done a decent job here
  • Should this be mentioned in the readme? I mentioned it for documentation purposes
  • Should this be mentioned in the --help output? The existing --really-delete-all-piv-keys flag is not there so I skipped it
  • Should there be advice that when this option is used it's better to do it on a clean system, on a trusted machine, offline and airgapped ideally, and any potential backups should be encrypted and stored securely?
  • Should the main agent daemon always handle keys that fail attestation (caused by generating on the computer instead of on the device) like I added here or maybe make this configurable with a switch?

Closes GH-65.

[1] go-piv/piv-go#83 (comment)

This is to allow creating backups of the private key in case the YubiKey
gets lost or damaged. The word "insecurely" is appended to the name of
the switch and a long warning produced in order to make people who don't
know what they're doing turn away and stop what they're doing.

This is possible since piv-go 1.7.0[1] so I allowed myself to bump the
version of this requirement.

[1] go-piv/piv-go#83 (comment)
@jstasiak
Copy link
Author

jstasiak commented Jan 6, 2021

Oh, one more thing: I considered importing an existing key from a file but decided against it for simplicity.

@jstasiak
Copy link
Author

After some thinking I think I'll change the command line switch to an interactive prompt to choose between on-the-card generation and on-the-computer generation + import. I have a branch that adds importing existing keys + RSA support (for deterministic signatures needed for sshcrypt[1] and sasquatch[2] to work (ECDSA is not compatible with their approaches) and the number of command line switches would grow unwieldy.

[1] https://github.com/leighmcculloch/sshcrypt
[2] https://github.com/muesli/sasquatch

Base automatically changed from master to main February 11, 2021 12:14
@FiloSottile
Copy link
Owner

Thank you for the PR, but I don't want to support importing keys in -setup. yubikey-agent is a simple opinionated tool that makes these choices for the user.

I might consider fixes that make it possible to import keys with other tools and then use them with yubikey-agent, if they are small enough, but see #91.

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.

UX/Roadmap question: importing existing keys
2 participants