[RFC] ecdsa: Add support for ECDH1-DERIVE method#70
[RFC] ecdsa: Add support for ECDH1-DERIVE method#70doanac wants to merge 2 commits intoThalesGroup:masterfrom
Conversation
|
Thank you @doanac I'll review this early next week and do a bit of testing on some physical HSMs. |
|
|
||
| // Perform CKM_ECDH1_DERIVE function with the given public key | ||
| func (c *Context) ECDH1Derive(pk Signer, pubkey *ecdsa.PublicKey) ([]byte, error) { | ||
| ecpk := pk.(*pkcs11PrivateKeyECDSA) |
There was a problem hiding this comment.
Could you add a checking type assertion here and if pk is not an instance of *pkcs11PrivateKeyECDSA return an appropriate error. It would be preferable not to panic.
Also can you move this function above pkcs11PrivateKeyECDSA Sign to keep Context functions together.
There was a problem hiding this comment.
I've done a force push to address these comments.
| return signer.context.dsaGeneric(signer.handle, pkcs11.CKM_ECDSA, digest) | ||
| } | ||
|
|
||
| // Perform CKM_ECDH1_DERIVE function with the given public key |
There was a problem hiding this comment.
Can you add some extra commentary here to help the user understand this function does not provide support for ANSI X9.63 ECDH derivation.
There was a problem hiding this comment.
i apologize - this is getting to a level of cryptography I'm not sure I can explain correctly. Maybe you can help me explain this nuance as I'm not sure I understand how it differs?
There was a problem hiding this comment.
not sure if it helps, but I used this logic as my guide:
https://github.com/OpenSC/OpenSC/blob/master/src/tools/pkcs11-tool.c#L3693
and
There was a problem hiding this comment.
If you look at https://www.cryptsoft.com/pkcs11doc/STANDARD/pkcs-11v2-20.pdf section 12.3.8 the specification allows for 2 different KDF-defining parameters; CKD_NULL as used here and CKD_ SHA1_KDF which can be used to derive keys inline with the ANSI X9.63 spec. The comment simply needs to reflect that this function makes no provision to support ANSI X9.63 key derivation
There was a problem hiding this comment.
thanks for the help! i've force pushed a new version with an updated comment
This is a temporary work-around while the PR gets sorted out: ThalesGroup/crypto11#70 Signed-off-by: Andy Doan <andy@foundries.io>
This is a temporary work-around while the PR gets sorted out: ThalesGroup/crypto11#70 Signed-off-by: Andy Doan <andy@foundries.io>
This method can be used to help do ecies encryption. Signed-off-by: Andy Doan <andy@foundries.io>
@doanac do you think you could add a test that performs the key derivation, does some operation with the key such as encrypt a random byte slice then use the go crypto libraries (or other) that does the reverse operation and verifies the decrypted byte slice matches? |
|
I'd be happy to add a test. I'm a little swamped today, but can probably get it done tomorrow. |
Signed-off-by: Andy Doan <andy@foundries.io>
|
@nickrmc83 - I've added a test that creates a known private key. I can then do ECDHDerive logic with the two keys to assert they both produce the same answer. While doing this I noticed that |
|
@doanac Once the CI passes this is good to go. |
|
@doanac can you take a look at the CI failure? You should be able to recreate it locally using |
|
FWIW, this patch rebased cleanly on top of |
4 participants
Thanks for the excellent library. It was pretty easy for me to get go's httpclient to use this and softhsm for TLS.
Now I'm trying something slightly more complex. I'm working with something that does symmetric encryption using EC keys based on ecies. The library I'm using assumes access to the private key:
https://github.com/ethereum/go-ethereum/blob/master/crypto/ecies/ecies.go#L120-L138
However, the operation done in that function is the equivalent of ECDH1-DERIVE. This PR is a proof-of-concept that allows me to do ecies decryption. I'm hoping you might be open to this change or something like it.