go-piv / piv-go

Keys and certificates for YubiKeys, written in Go
Apache License 2.0
366 stars 65 forks source link

ECDH for using P-384 keys for more than signing #79

Closed tv42 closed 4 years ago

tv42 commented 4 years ago

go-ykpiv's CGo wrapper of libykpiv has this:

https://godoc.org/github.com/go-piv/go-ykpiv#Slot.Decrypt

f the slot holds an EC key, then we will perform ECDH and return the shared secret. In this case, msg must be the peer's public key in octet form, as specified in section 4.3.6 of ANSI X9.62.

I've tested that to work. I would, however, like to use pure-Go libraries.

I couldn't figure out how to get ECDH working with piv-go. For the CGo version, sign and decrypt look very similar:

https://github.com/go-piv/go-ykpiv/blob/32315ce599d8e21e91880e6ecdce55a98420ddc0/decrypt.go#L68
https://github.com/go-piv/go-ykpiv/blob/32315ce599d8e21e91880e6ecdce55a98420ddc0/sign.go#L166

and so do libykpiv's two functions, they both call the same function:

https://github.com/Yubico/yubico-piv-tool/blob/0b27308560ab4f3df4bc29a179e1c91649ce0629/lib/ykpiv.c#L1144
https://github.com/Yubico/yubico-piv-tool/blob/0b27308560ab4f3df4bc29a179e1c91649ce0629/lib/ykpiv.c#L1126

go-ykpiv has a helpful test case here:

https://github.com/go-piv/go-ykpiv/blob/32315ce599d8e21e91880e6ecdce55a98420ddc0/ykpiv_test.go#L522

But I can't see where exactly signing and decrypting are different to make ECDH work with go-piv. Any thoughts?

tv42 commented 4 years ago

A-ha! Found the difference! decipher is the final argument to _general_authenticate and the code branches based on it:

https://github.com/Yubico/yubico-piv-tool/blob/0b27308560ab4f3df4bc29a179e1c91649ce0629/lib/ykpiv.c#L1048-L1052

https://github.com/Yubico/yubico-piv-tool/blob/0b27308560ab4f3df4bc29a179e1c91649ce0629/lib/ykpiv.c#L1064

tv42 commented 4 years ago

I tried reproducing the 0x81->0x85 difference here

https://github.com/go-piv/piv-go/blob/d3b05b2e6319f2c914f9615df79fdc4a662a2840/piv/key.go#L756

but that just broke with

command failed: smart card error 6a80: incorrect parameter in command data field

Same yubikey, same slot, works with ykpiv. I must be still doing something wrong.

tv42 commented 4 years ago

Ah, signing was silently truncating the input still. Commenting out this line kludges ykSignECDSA into a ECDH function.

https://github.com/go-piv/piv-go/blob/d3b05b2e6319f2c914f9615df79fdc4a662a2840/piv/key.go#L746

tv42 commented 4 years ago

Now, obviously the above is not the right way to do it, just a proof of concept.

How should the real API look like?

I'm not thrilled by the idea of a Decrypt that doesn't really decrypt. Should YubiKey.PrivateKey return value learn an extra optional interface?

type KeyAgreement interface {
    // Perform a Diffie-Hellman key agreement with the peer.
    //
    // Peer's public key must use the same algorithm as
    // the key in this slot, or returns error ErrMismatchingAlgorithms.
    KeyAgreement(peer crypto.PublicKey) ([]byte, error)
}
tv42 commented 4 years ago

I guess that KeyAgreement should take rand io.Reader and opts crypto.SignerOpts too, even if ECDSA doesn't use them?

Any thoughts on the name of the interface, can't -er name it easily. KeyAgreementer. KeyAgreer. Are there good synonyms for the Diffie-Hellman operation? I already intentionally made it not say EC or DH, because I thought both of those were algorithm-specific details.

tv42 commented 4 years ago

I'll have a pull request ready as soon as I figure out how this library is tested.

tv42 commented 4 years ago

First stab is at https://github.com/tv42/piv-go/tree/wip-ecdh, without tests for now.