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

x/crypto/nacl: Support for libsodium "sealed box" #35346

Closed
btoews opened this issue Nov 4, 2019 · 13 comments
Closed

x/crypto/nacl: Support for libsodium "sealed box" #35346

btoews opened this issue Nov 4, 2019 · 13 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Proposal-Accepted Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone

Comments

@btoews
Copy link

btoews commented Nov 4, 2019

The x/crypto/nacl package doesn't currently implement the libsodium "sealed box" primitive (docs). This functionality is a very lightweight extension of the functionality provided by x/crypto/nacl/box, providing anonymous encryption using the receiver's public key (encrypt a message to the receiver without the sender having their own keypair).

My inclination would be to create another package — x/crypto/nacl/sealedbox — exposing this functionality. My only concern with this approach is that key generation between box and sealedbox is identical. Would it make sense to expose a separate sealedbox.GenerateKey() function or to simply instruct the user to call box.GenerateKey()?

The other option would be to build this functionality into the box package, providing SealAnonymous() and OpenAnonymous() functions.

/cc @FiloSottile

@gopherbot gopherbot added this to the Proposal milestone Nov 4, 2019
@FiloSottile
Copy link
Contributor

I think I like adding APIs to x/crypto/nacl/box better, if nothing else because the real box API is awkward, and adding the much more useful anonymous options next to it would help users who are confused about how to use box.

@FiloSottile FiloSottile added the Proposal-Crypto Proposal related to crypto packages or other security issues label Nov 4, 2019
@btoews
Copy link
Author

btoews commented Nov 4, 2019

Sounds good to me. In that case, I propose adding two new functions to the box package

func SealAnonymous(out, message []byte, peersPublicKey *[32]byte) []byte {}
func OpenAnonymous(out, box []byte, privateKey *[32]byte) ([]byte, bool) {}

The nonce generation will be calling into blake2b functions that may return errors. In practice, errors are only returned from those functions if a bad hash size is specified. Since we know the hash size will be valid, I propose panicking if an error is encountered, rather than surfacing them to the caller.

@btoews
Copy link
Author

btoews commented Nov 4, 2019

This actually might be a bit more complicated. The ephemeral keypair needs to be generated randomly. So, SealAnonymous should also probably take a rand io.Reader argument. Since reading this can fail, I suppose SealAnonymous should also return an error? It's rather unfortunate to add this complexity to what should be a simple API.

@FiloSottile
Copy link
Contributor

Alas, there's a lot of unfortunate things in that API: bool instead of error, array pointers, append instead of inlined allocation... but I guess we should be consistent, so yeah.

func SealAnonymous(out, message []byte, recipient *[32]byte, rand io.Reader) ([]byte, error)
func OpenAnonymous(out, box []byte, key *[32]byte) (message []byte, ok bool)

I am tempted by the more idiomatic SealWithoutSender name, but I guess we should be consistent with libsodium's naming.

A nil rand should default to crypto/rand.Reader.

@btoews
Copy link
Author

btoews commented Nov 4, 2019

Sounds good. Do I need to write a design doc for this? Do I need to wait for an official approval here, or can I go ahead with a PR?

@FiloSottile
Copy link
Contributor

Feel free to open a CL, we should leave this issue open for a week or so before marking it accepted to give time to the community to comment.

@rsc
Copy link
Contributor

rsc commented Nov 27, 2019

For what it's worth, please let the proposal review meetings add Proposal-FinalCommentPeriod so that anything in final comment period is listed in the minutes (golang.org/s/proposal-minutes). We'll list it this week. Thanks.

@rsc
Copy link
Contributor

rsc commented Nov 27, 2019

Based on the discussion above this seems like a likely accept.
Will add to the proposal minutes and leave open for another week for comments.

@FiloSottile
Copy link
Contributor

Something I didn't realize is that the OpenAnonymous API requires the local public key to generate the nonce. It can be generated from the private key, but that's quite the performance overhead. It's also too late to make the private key carry both.

https://go-review.googlesource.com/c/crypto/+/205241/1/nacl/box/box.go#156

libsodium takes the public key from the caller.

int crypto_box_seal_open(unsigned char *m, const unsigned char *c,
                         unsigned long long clen,
                         const unsigned char *pk, const unsigned char *sk);

I kind of hate that they are two *[32]byte that are easy to swap, but the API cleanliness probably doesn't justify an extra scalar multiplication at every decryption, basically doubling the fixed overhead.

func OpenAnonymous(out, box []byte, publicKey, privateKey *[32]byte) (message []byte, ok bool)

The only alternative I see is to fork the API entirely, with a new GenerateKey that returns a bundled private+public key, at that point maybe in a new package, like golang.org/x/crypto/box/anonymous? It'd be a good opportunity to use slices rather than byte arrays, return slices instead of appending, and return error values, too.

Opinions?

(@rsc, yep, will do that going forward. I think I marked this one before we discussed it.)

@btoews
Copy link
Author

btoews commented Dec 3, 2019

My vote would be to take the public key as an argument. We could even make it optional and calculate the public key if it isn't provided. It seems less than ideal to have a separate package with a new key type that isn't interoperable with the existing keys.

@gopherbot
Copy link

Change https://golang.org/cl/205241 mentions this issue: nacl/box: support anonymous seal/open

@FiloSottile
Copy link
Contributor

Alright, let's go for the libsodium-like API.

func SealAnonymous(out, message []byte, recipient *[32]byte, rand io.Reader) ([]byte, error)
func OpenAnonymous(out, box []byte, publicKey, privateKey *[32]byte) (message []byte, ok bool)

@rsc rsc added this to Incoming in Proposals (old) Dec 4, 2019
@rsc
Copy link
Contributor

rsc commented Dec 4, 2019

No change in consensus, so accepted.

@rsc rsc changed the title Proposal: x/crypto/nacl: Support for libsodium "sealed box" x/crypto/nacl: Support for libsodium "sealed box" Dec 4, 2019
@rsc rsc moved this from Incoming to Likely Accept in Proposals (old) Dec 4, 2019
@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Dec 11, 2019
@rsc rsc removed the Proposal label Dec 11, 2019
@rsc rsc modified the milestones: Proposal, Unreleased Dec 11, 2019
@rsc rsc moved this from Likely Accept to Accepted in Proposals (old) Dec 11, 2019
@golang golang locked and limited conversation to collaborators Dec 18, 2020
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 28, 2022
This adds SealAnonymous and OpenAnonymous functions that implement the
libsodium "sealed box" functionality.

Fixes golang/go#35346

Change-Id: I22455f1b83595ec8a68d1861e635bd6cb0573f44
GitHub-Last-Rev: 7d334cf861942ec63ad613b7f28fb6dd7a1f9992
GitHub-Pull-Request: golang/crypto#107
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/205241
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
This adds SealAnonymous and OpenAnonymous functions that implement the
libsodium "sealed box" functionality.

Fixes golang/go#35346

Change-Id: I22455f1b83595ec8a68d1861e635bd6cb0573f44
GitHub-Last-Rev: 7d334cf861942ec63ad613b7f28fb6dd7a1f9992
GitHub-Pull-Request: golang/crypto#107
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/205241
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
This adds SealAnonymous and OpenAnonymous functions that implement the
libsodium "sealed box" functionality.

Fixes golang/go#35346

Change-Id: I22455f1b83595ec8a68d1861e635bd6cb0573f44
GitHub-Last-Rev: 7d334cf861942ec63ad613b7f28fb6dd7a1f9992
GitHub-Pull-Request: golang/crypto#107
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/205241
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
This adds SealAnonymous and OpenAnonymous functions that implement the
libsodium "sealed box" functionality.

Fixes golang/go#35346

Change-Id: I22455f1b83595ec8a68d1861e635bd6cb0573f44
GitHub-Last-Rev: 7d334cf861942ec63ad613b7f28fb6dd7a1f9992
GitHub-Pull-Request: golang/crypto#107
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/205241
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
BiiChris pushed a commit to BiiChris/crypto that referenced this issue Sep 15, 2023
This adds SealAnonymous and OpenAnonymous functions that implement the
libsodium "sealed box" functionality.

Fixes golang/go#35346

Change-Id: I22455f1b83595ec8a68d1861e635bd6cb0573f44
GitHub-Last-Rev: 7d334cf
GitHub-Pull-Request: golang#107
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/205241
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Proposal-Accepted Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants