Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

preliminary ECDH support #37

Merged
merged 1 commit into from
Oct 4, 2016
Merged

preliminary ECDH support #37

merged 1 commit into from
Oct 4, 2016

Conversation

maxtaco
Copy link

@maxtaco maxtaco commented Oct 3, 2016

  • make sure that we can import ECDH private keys
  • make some test cases
  • write out some test data
  • start on actual ECDH decryption, but the crux is unimplemented

- make sure that we can import ECDH private keys
- make some test cases
- write out some test data
- start on actual ECDH decryption, but the crux is unimplemented
@maxtaco
Copy link
Author

maxtaco commented Oct 3, 2016

@oconnor663 can you take a look?

This allows us to import and pass through ECDH keys, but I didn't implement decryption with ECDH keys.

@maxtaco
Copy link
Author

maxtaco commented Oct 3, 2016

With this fix, we're now able to run keybase pgp select on keys that have ECDH subkeys (over Nist or Brainpool curves). Whereas before we weren't

@@ -366,6 +374,24 @@ func (pk *PrivateKey) parseElGamalPrivateKey(data []byte) (err error) {
return nil
}

func (pk *PrivateKey) parseECDHPrivateKey(data []byte) (err error) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function looks like it's a duplicate of the ones around it. It looks like the only difference between them is the types they cast into? Is that a "don't fix what ain't broke" thing?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it at least will allow reading of the key without crashing (what we used to do with an actual panic). I ought to go ahead and implement ECDH but it's tricky and takes a lot of tinkering, so I think it's worth it to put off for now, and at least get import/export of these keys working.

@oconnor663
Copy link

Looks fine to me, with the one question above.

@maxtaco maxtaco merged commit 93f5b35 into master Oct 4, 2016
@maxtaco maxtaco deleted the maxtaco/CORE-3806 branch October 4, 2016 15:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants