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

Proposal for nacl.sign API change (next version) #36

Closed
dchest opened this issue Jul 15, 2014 · 9 comments
Closed

Proposal for nacl.sign API change (next version) #36

dchest opened this issue Jul 15, 2014 · 9 comments
Assignees

Comments

@dchest
Copy link
Owner

dchest commented Jul 15, 2014

I'd like to change the signature API sometime in the future to this:

nacl.sign(message, secretKey) -> signedMessage
nacl.sign.open(signedMessage, publicKey) -> message | null

(Maybe instead of null, return false when verification fails? null makes more sense to me in this case)

Basically, this is how signatures in NaCl/TweetNaCl work.

But since everyone likes to have signatures separate from messages ("detached signatures"), we will add the following new methods to get the behavior similar, but not identical, to what we currently have:

nacl.sign.detached(message, secretKey) -> signature
nacl.sign.detached.verify(message, signature, publicKey) -> true | false

Note that it's verify and not open, because it returns only true or false, not the "unsigned" message or false like nacl.sign.open.

This is inspired by libsodium.

Compatibility

This would require major version bump (to 1.0.0, argh) according to semver principles.

To deliberately break apps which upgraded to 1.0.0 without changing code, we can throw exception if nacl.sign.open is called with 3 arguments. We can do nothing with nacl.sign, though: instead of just signature it will return signed message. This isn't dangerous. I don't think there's enough users right now anyway to cause headaches :)

Comments?

@dchest dchest self-assigned this Jul 15, 2014
@dchest dchest changed the title API change for nacl.sign (next version) API change for nacl.sign proposal (next version) Jul 15, 2014
@dchest dchest changed the title API change for nacl.sign proposal (next version) Proposal for nacl.sign API change (next version) Jul 15, 2014
@dchest
Copy link
Owner Author

dchest commented Jul 16, 2014

According to semver spec point 4:

Major version zero (0.y.z) is for initial development. Anything may change at any time. The public
API should not be considered stable.

So it will probably be v0.10.0 instead of v1.0.0, yay.

@devi
Copy link
Contributor

devi commented Jul 17, 2014

Can we improve nacl.sign.keyPair.fromSecretKey ?

Current implementation is a bit confusing to me.
Since it doesn't generate keypair, does it mean extracting public key from secret key ?

May be we can have something like crypto_sign_seed_keypair from libsodium.

@dchest
Copy link
Owner Author

dchest commented Jul 17, 2014

@devi yes, it just extracts the public key, not generates it. I didn't name it "extract" because while in sign case we only copy it from the secret key, in nacl.box.keyPair.fromSecretKey we actually calculate it from the secret key using scalar multiplication.

Probably needs a better documentation rather than API change, but I'm open to new names.

@dchest
Copy link
Owner Author

dchest commented Jul 17, 2014

As for crypto_sign_seed_keypair: I need to think about it.

On one hand, there's currently no way to do what this function does without copying half of the library, on the other hand, it complicates code (and makes diff from tweenacl.c not so pretty) for an uncommon use case (which can also be dangerous if you're not careful with producing proper seeds, e.g. from passwords with weak derivation function).

@devi
Copy link
Contributor

devi commented Jul 17, 2014

It won't be necessary to change names.
I'm just thinking, it would be nice if we could generate public key like nacl.box.keyPair.fromSecretKey did.

@devi
Copy link
Contributor

devi commented Jul 17, 2014

Internet connection is killing me. Didn't saw your comment regarding crypto_sign_seed_keypair.
That's exactly what I'm thinking.

@dchest
Copy link
Owner Author

dchest commented Jul 17, 2014

@devi opened a new issue #37 for it.

dchest added a commit that referenced this issue Jul 22, 2014
Previously, nacl.sign returned a signature, and nacl.sign.open accepted
a message and "detached" signature. This was unlike NaCl's API, which
dealt with signed messages (concatenation of signature and message).

The new API is:

  nacl.sign(message, secretKey) -> signedMessage
  nacl.sign.open(signedMessage, publicKey) -> message | null

(Compatibility: the latter will throw exception if it doesn't receive 2
arguments, so that it will deliberately break code that uses old API,
but the former function won't be able to detect this, and users who
didn't upgrade their code will receive a signed message instead of just
a signature.)

Since detached signatures are common, two new API functions are
introduced:

  nacl.sign.detached(message, secretKey) -> signature
  nacl.sign.detached.verify(message, signature, publicKey) -> true | false

Due to change of API, we increment minor version: from 0.9.2 to 0.10.0
according to semver spec point 4 (for 0.x.y versions).

This change was discussed in issue #36.
@dchest
Copy link
Owner Author

dchest commented Jul 22, 2014

PR #40 will be used for further discussions (if any), so closing this now.

@dchest dchest closed this as completed Jul 22, 2014
dchest added a commit that referenced this issue Jul 22, 2014
Previously, nacl.sign returned a signature, and nacl.sign.open accepted
a message and "detached" signature. This was unlike NaCl's API, which
dealt with signed messages (concatenation of signature and message).

The new API is:

  nacl.sign(message, secretKey) -> signedMessage
  nacl.sign.open(signedMessage, publicKey) -> message | null

(Compatibility: the latter will throw exception if it doesn't receive 2
arguments, so that it will deliberately break code that uses old API,
but the former function won't be able to detect this, and users who
didn't upgrade their code will receive a signed message instead of just
a signature.)

Since detached signatures are common, two new API functions are
introduced:

  nacl.sign.detached(message, secretKey) -> signature
  nacl.sign.detached.verify(message, signature, publicKey) -> true | false

Due to change of API, we increment minor version: from 0.9.2 to 0.10.0
according to semver spec point 4 (for 0.x.y versions).

This change was discussed in issue #36.
dchest added a commit that referenced this issue Jul 22, 2014
Previously, nacl.sign returned a signature, and nacl.sign.open accepted
a message and "detached" signature. This was unlike NaCl's API, which
dealt with signed messages (concatenation of signature and message).

The new API is:

  nacl.sign(message, secretKey) -> signedMessage
  nacl.sign.open(signedMessage, publicKey) -> message | null

(Compatibility: the latter will throw exception if it doesn't receive 2
arguments, so that it will deliberately break code that uses old API,
but the former function won't be able to detect this, and users who
didn't upgrade their code will receive a signed message instead of just
a signature.)

Since detached signatures are common, two new API functions are
introduced:

  nacl.sign.detached(message, secretKey) -> signature
  nacl.sign.detached.verify(message, signature, publicKey) -> true | false

Due to change of API, we increment minor version: from 0.9.2 to 0.10.0
according to semver spec point 4 (for 0.x.y versions).

This change was discussed in issue #36.
@dchest
Copy link
Owner Author

dchest commented Jul 26, 2014

Merged.

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

No branches or pull requests

2 participants