Add support for ECDH key agreement in Transit Secret Engine #811
Conversation
Signed-off-by: DanGhita <[email protected]>
Signed-off-by: DanGhita <[email protected]>
Signed-off-by: DanGhita <[email protected]>
Signed-off-by: DanGhita <[email protected]>
| @@ -161,6 +162,14 @@ func (kt KeyType) DerivationSupported() bool { | |||
| return false | |||
| } | |||
|
|
|||
| func (kt KeyType) KeyAgreementSupported() bool { | |||
There was a problem hiding this comment.
Little surprised you don't support Curve25519 EdDH derivation :-)
There was a problem hiding this comment.
Strange, my previous comment seems to be lost :(
I was saying: OK, I will try to add KeyType_ED25519, since they are pretty used in the field.
However, if I'm not wrong, they are considered as less secure than the other curves; for example, ANSSI (French Security Agency) considers ED25519 just as an "acceptable" alternative.
Furthermore, digging into the subject, it seems that most of crypto/elliptic functions are deprecated (so, probably I should change the current ECDH implementation):
https://pkg.go.dev/crypto/elliptic#P521
There was a problem hiding this comment.
NIST P-Curves are vulnerable to certain problems that Ed25519 isn't: https://safecurves.cr.yp.to/
NIST recently certified EdDSA at the same level as ECDSA in FIPS 186-5.
There was a problem hiding this comment.
I think switching to crypto/ecdh would be a good move since it supports both NIST P-Curves and Ed25519.
There was a problem hiding this comment.
Yes, that was my idea too, I will play a little bit with the crypto/ecdh API and if everything works fine, I will update my implementation accordingly. Thank you @fatima2003 !
| * ecdh | ||
| Defaults to "ecdh".`, | ||
| }, | ||
|
|
There was a problem hiding this comment.
Maybe there needs to be a kdf parameter, which accepts "" or "hkdf", and if using HKDF, can add HKDF-specific parameters (like salt or context)? (to answer your TODO below).
Then HKDF can also be a key type potentially, and we can do multiple HKDF invocations (without doing multiple ECDH invocations), if they don't want to use the shared secret directly.
Resolves #655
Target Release
1.14.7