Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Ed25519 signing support #206

Closed
briansmith opened this issue May 30, 2016 · 6 comments
Closed

Add Ed25519 signing support #206

briansmith opened this issue May 30, 2016 · 6 comments

Comments

@briansmith
Copy link
Owner

briansmith commented May 30, 2016

This should be relatively easy because there's good code in crypto/curve25519/curve25519.c that can be used. Actually, in src/ec/eddsa.rs, there is already a function named ed25519_sign that implements the signing. So, the bulk of this work is just implementing the ring::signature::SigningKey API that is consistent with the rest of ring, and then adding ring::signature::ED25519_SIGNING.

The tricky part is that there's no code to load a serialized private key, and there's no way to serialize a private key, which would be important to many uses. Deserialization and serialization could be done as a follow-on.

See also any general notes about signing APIs in #205.

@briansmith briansmith mentioned this issue May 30, 2016
5 tasks
@djc
Copy link
Contributor

djc commented Jun 2, 2016

So without ser/de, what does the SigningKey instantiation look like? Just a 64-element u8 slice?

@briansmith
Copy link
Owner Author

Should it be a 64-element u8 slice, or two 32-element u8 slices, one for the private part and one for the public part?

Anyway, yes, at least until we have deserialization and serialization support, setting the key via a slice or slices is fine.

@briansmith
Copy link
Owner Author

If you're interested in working on this, I suggest that you just try to implement SigningKey in the most simple way possible, hard-coding Ed25519 (and SHA-512). Then we can evolve the API from there.

For example, instead of trying to define a generic SigningKey type and a generic sign function, we can have an Ed25519KeyPair struct with methods like this:

// A wrapper around an array of bytes, very much like digest::Digest.
struct Signature {
    ...
}

impl Ed25519KeyPair<'a> {
    fn generate(rng: &SecureRandom) -> Result<Ed25519KeyPair, ()>,
    fn from_bytes(private_key: &[u8], public_key: &[u8]) -> Result<Ed25519KeyPair, ()>,
    fn private_key_bytes() -> &'a [u8],
    fn public_key_bytes() -> &'a [u8],
    fn sign(&self, msg: Input) -> Signature,
}

@briansmith
Copy link
Owner Author

briansmith commented Jun 4, 2016

Note: Ed25519KeyPair and all the code should be in ring::ec::eddsa. Then, ring::signature should pub use ec::eddsa::Ed25519KeyPair to expose it in the public API.

[Edit: s/agreement/signature/]

@briansmith
Copy link
Owner Author

briansmith commented Jun 4, 2016

Rough Steps:

  • Implement Ed25519KeyPair
  • Tests (I think there are already tests existing, using the private ed25519_sign function; they may just need to be adapted.)
  • Add documentation, including a full example. (Note: the example should use signature::Ed25519SKeyPair, not ec::eddsa::Ed25519KeyPair. Also, you need to temporarily flip the doctests on in Cargo.toml; they are disabled because they brea cross-compiling.)
  • Benchmarks in crypto-bench.

[Edit: s/agreement/signature/]

@briansmith briansmith changed the title Add EdDSA signing support Add Ed25519 signing support Jun 5, 2016
djc added a commit to djc/ring that referenced this issue Jun 5, 2016
I agree to license my contributions to each file under the terms given
at the top of each file I changed.
@briansmith
Copy link
Owner Author

I'm going to close this, since the only thing left on the checklist is the benchmarks, and crypto-bench already is keeping track of what needs to be done. Thanks again @djc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants