Implement import key functionality

[hayleyjames]

Issue #77 discusses whether this library should support this feature. My use case for implementing this is to be able to create backup Yubikeys with the same private key.

Tested on: Ubuntu 20.04.1 LTS

[ericchiang]

For consistency this should probably be "SetPrivateKey"

Since there's already a struct that holds the policies, can the arguments be:

func (yk *YubiKey) SetPrivateKey(key [24]byte, slot Slot, priv crypto.PrivateKey, policy Key)

Or we can refactor this to:

type KeyPolicy struct {
    PINPolicy PINPolicy
    TouchPolicy TouchPolicy
    // Opt-in required to use imported private keys?
    InsecureAllowImportedPrivateKeys bool
}
func (yk *YubiKey) SetPrivateKey(key [24]byte, slot Slot, priv crypto.PrivateKey, policy KeyPolicy)

I can't review the rest without a link to the spec. You can see a few comments that have links to pages in the NIST pdf. Can you include the one that defines the key parameters:

https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-73-4.pdf#page=95

Finally, I think there should be warnings associated with this method:

// SetPrivateKey is an insecure method which imports a private key into the slot.
// Users should almost always use GeneratePrivateKey() instead.
//
// Importing a private key breaks functionality provided by this package, including
// AttestationCertificate() and Attest(). There are no stability guarantees for other
// methods for imported private keys.
//
// Keys generated outside of the YubiKey should not be considered hardware-backed,
// as there's no way to prove the key wasn't copied, exfiltrated, or replaced with malicious
// material before being imported.
func (yk *YubiKey) SetPrivateKey(...)

[ericchiang]

Also, wanted to say thanks for the PR! Apologies for some if some of the "insecure" comments seem dramatic. For security relevant code, I always find it helpful to optimize for reviewers (so they can see an "Insecure..." variable) and developers who are approaching this for the first time.

[hayleyjames]

Thank you for reviewing my PR!

Adding an insecure warning is a good idea. The command is a Yubico extension to PIV. You can find the yubico-piv-tool implementation here and the documentation here.

I don't think an InsecureAllowImportedPrivateKeys flag would work because we can't differentiate between an empty slot and an imported key.

Would naming the function SetPrivateKeyInsecure be appropriate?

[ericchiang]

"SetPrivateKeyInsecure" works for me

[ericchiang]

Tests passed on my Macbook.

lgtm. Can you squash your changes into a single commit? I'll merge after.

https://stackoverflow.com/a/5201642

[hayleyjames]

Thanks for reviewing my changes! The tests also pass on Windows 10.

[ericchiang]

Thanks for your contribution! This has been included in the v1.7.0 tag https://github.com/go-piv/piv-go/releases/tag/v1.7.0