#3 closed enhancement (wontfix)

serialize ecdsa keys without the fluff

Reported by: zooko Owned by:
Priority: major Milestone:
Version: 0.4.0 Keywords:
Cc: Launchpad Bug:

Description

The current pycryptopp (v0.4) serializes keys by using Crypto++'s default encoding, which is DER encoding (meaning a couple of bytes of tag and length information), and a version number, and for ecdsa public keys includes the y-value of the point.

Make a minimal, just-the-bytes-we-really-need encoding for ECDSA private keys and public keys.

Attachments (3)

diff-ecdsa.txt (39.7 KB) - added by zooko at 2009-03-03T04:29:17Z.
diff.patch.txt (25.1 KB) - added by zooko at 2009-03-24T04:40:49Z.
version of this patch which attempts to use the old technique of the Python object having a pointer to the C++ object, instead of the new and very segfaulty technique of having the Python object contain the C++ object directly
ticket3-close.darcspatch (6.8 KB) - added by nejucomo at 2009-09-01T15:44:36Z.
Patch to re-enable ecdsa and add a unit test for serialization length.

Download all attachments as: .zip

Change History (16)

comment:1 Changed at 2008-04-03T15:48:05Z by zooko

  • Status changed from new to accepted

Oh, actually pycryptopp v0.4 is using the default encoding which adds such potentially useful information as the public key (when you encode the private key), the algorithm ID, the parameters, etc. So currently 192-bit ECDSA private keys serialize to 230 bytes, and 521-bit private keys serialize to 527 bytes.

comment:2 Changed at 2008-05-01T11:52:13Z by zooko

  • Resolution set to fixed
  • Status changed from accepted to closed

done in [550]

comment:3 Changed at 2008-05-01T11:53:41Z by zooko

  • Resolution fixed deleted
  • Status changed from closed to reopened

Oh, except the ecdsa public keys are still serialized in the fluffy way, so this ticket isn't done.

comment:4 Changed at 2008-05-06T23:35:13Z by warner

Did you mean that 192-bit private keys are serialized to 230 bits ? 230 bytes sounds like an amazing amount of additional data :-).

comment:5 Changed at 2008-05-15T12:44:18Z by zooko

http://allmydata.org/trac/tahoe/ticket/331 (add DSA to pycryptopp) is blocked on this ticket.

comment:6 Changed at 2009-02-04T02:17:23Z by warner

You might want to consider the semi-private-keys API suggestion in #13 when doing this work.. it isn't necessary to provide semi-private keys right away, but if you like the #13 API, you could leave room for the extra constructor arguments so semi-private keys can be added in a backwards-compatible fashion later.

comment:7 Changed at 2009-03-02T23:29:32Z by warner

Zooko: if you will decide on what the API will be (maybe considering #13, maybe not), I will fix up the unit tests to match, so you'll have an easy way to know when you're finished coding.

comment:8 Changed at 2009-03-03T00:29:34Z by warner

ooh, http://www.cryptopp.com/wiki/Elliptic_Curve_Cryptography has an example (copied from the mailing list, apparently) of minimal-size serialization of EC (encryption) keys.. might be useful.

comment:9 Changed at 2009-03-03T04:28:57Z by zooko

Here's a patch from my sandbox that does this as well as #2 (deterministic generation of private key from small seed). There are a few problems with this patch:

  1. There's something terribly wrong with the memory management, so that it segfaults. I'm pretty sure that this has to do with my attempt to embed a C++ object (defined by a Crypto++ class) directly into a Python object (a struct that starts with the Python object header fields). The motivation for this is improved efficiency and (ha!) safer memory management. I intend to separate out this attempt from the rest of the patch and try to make it work without that feature.
  1. It builds its own deterministic key generation RNG using Tiger. Recently on the cryptopp mailing list Wei Dai explained that a stream cipher such as AES-CTR can be used as an RNG, although I'm afraid it might work only in the newest, not-yet-released version of Crypto++: http://groups.google.com/group/cryptopp-users/browse_thread/thread/9e02130e55988500#

Changed at 2009-03-03T04:29:17Z by zooko

comment:10 Changed at 2009-03-24T04:36:33Z by zooko

Here's a patch which is the previous patch minus the feature of embedding C++ objects directly into Python objects. (If you are interested, observe how there are more conditions in this patch which can lead to PyErr_NoMemory(), while doing so, please see if there are some conditions in which allocation can fail but I forgot to test for it. Those can be exploitable bugs in some cases, and I found another one just now while working on this patch.)

This patch is probably buggy as all hell. I'm too sleepy to test it. By the way, I've been observing how running the pycryptopp unit tests from trunk pycryptopp reliably give me a segfault in pycryptopp.test.test_rsa.SignAndVerify.test_serialize_and_deserialize_signing_key_and_test on yukyuk (linux-amd64) but not on ootles (MacOSX-amd64), and how the reliable segfault disappears when I try to run it under valgrind. Also I occasionally see segfaults from other programs on yukyuk -- I can see one from okular in my syslog from earlier today. memtest86 always passes on yukyuk. I'll probably experiment with different linux kernels, or pouring cold water over the CPU, or something tomorrow.

Changed at 2009-03-24T04:40:49Z by zooko

version of this patch which attempts to use the old technique of the Python object having a pointer to the C++ object, instead of the new and very segfaulty technique of having the Python object contain the C++ object directly

Changed at 2009-09-01T15:44:36Z by nejucomo

Patch to re-enable ecdsa and add a unit test for serialization length.

comment:11 Changed at 2009-09-01T15:47:18Z by nejucomo

Someone with more context should review this ticket and close it.

I've attached a patch which re-enables the ecdsa module contents, unit tests, and build config. Additionally, it replaces a bare "25" with a named constant in the module source and adds a unit test ensuring the serialization is exactly 25 bytes. (This unit test will fail for different key sizes!)

With these patches len(myVerifier.serialize()) == 25, which is 192 bits of key plus 1 byte of overhead. There is no fluff.

comment:12 Changed at 2010-01-06T16:12:49Z by zooko

  • Owner zooko deleted
  • Status changed from reopened to new

It sounds to me like nejucomo's patch will fix this ticket.

comment:13 Changed at 2013-01-30T17:33:10Z by zooko

  • Resolution set to wontfix
  • Status changed from new to closed

Superceded by Ed25519! Yay!

Note: See TracTickets for help on using tickets.