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.

Sign up for GitHub

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

Jump to bottom

proposal: crypto/cipher: detached mode AEAD Seal/Open #24990

Closed
riobard opened this issue Apr 22, 2018 · 15 comments
Closed

proposal: crypto/cipher: detached mode AEAD Seal/Open #24990

riobard opened this issue Apr 22, 2018 · 15 comments
Labels
FrozenDueToAge Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone
Proposal

Comments

@riobard
Copy link

riobard commented Apr 22, 2018 •
edited
Loading

The AEAD interface currently exposes only combined mode operation with authentication tag appended to the cipher text.

The popular crypto library libsodium supports detached mode operation where authentication tag and cipher text can be at different locations (e.g. some applications may need to store authentication tag before cipher text).

Due to Go1 compatibility requirement the existing AEAD interface cannot be changed. Therefore I propose we add a new interface DetachedAEAD.

type DetachedAEAD interface {
        // NonceSize returns the size of the nonce that must be passed to Seal
        // and Open.
        NonceSize() int

        // TagSize returns the size of the tag.
        TagSize() int

        // Seal encrypts and authenticates plaintext, authenticates the
        // additional data and appends the result to dst, returning the updated
        // slice. The nonce must be NonceSize() bytes long and unique for all
        // time, for a given key. The tag must be at least TagSize() bytes long.
        //
        // The plaintext and dst must overlap exactly or not at all. To reuse
        // plaintext's storage for the encrypted output, use plaintext[:0] as dst.
        Seal(dst, tag, nonce, plaintext, additionalData []byte) []byte

        // Open decrypts and authenticates ciphertext, authenticates the
        // additional data and, if successful, appends the resulting plaintext
        // to dst, returning the updated slice. The nonce must be NonceSize()
        // bytes long and both it and the additional data must match the
        // value passed to Seal. The tag must be at least TagSize() bytes long.
        //
        // The ciphertext and dst must overlap exactly or not at all. To reuse
        // ciphertext's storage for the decrypted output, use ciphertext[:0] as dst.
        //
        // Even if the function fails, the contents of dst, up to its capacity,
        // may be overwritten.
        Open(dst, tag, nonce, ciphertext, additionalData []byte) ([]byte, error)
}
@riobard riobard changed the title crypto/cipher: Detached mode AEAD Seal/Open crypto/cipher: detached mode AEAD Seal/Open Apr 22, 2018
@agnivade agnivade changed the title crypto/cipher: detached mode AEAD Seal/Open proposal: crypto/cipher: detached mode AEAD Seal/Open Apr 22, 2018
@gopherbot gopherbot added this to the Proposal milestone Apr 22, 2018
@agnivade agnivade added the Proposal-Crypto Proposal related to crypto packages or other security issues label Apr 22, 2018
@FiloSottile
Copy link
Contributor

Based on discussion with @agl, we want something like this. We need to decide what shape the API will take. If it will be different from BoringSSL we'll need a good reason to ignore their decision.

@FiloSottile FiloSottile mentioned this issue Jun 23, 2019
Closed
@jyxjjj
Copy link

jyxjjj commented Jun 23, 2019

@FiloSottile So, how to split at this moment? It seems to "append", so can i just get the last 32 length?

@FiloSottile
Copy link
Contributor

@jyxjjj Yes, but the GCM tag is normally 16 bytes. You can get its length by calling AEAD.Overhead().

@jyxjjj
Copy link

jyxjjj commented Jun 23, 2019

Thank you.

@rsc rsc moved this to Incoming in Proposals Aug 10, 2022
@rsc rsc added this to Proposals Aug 10, 2022
@rsc
Copy link
Contributor

rsc commented Aug 17, 2022

Adding to proposal minutes. Is this still something people need?

@rsc
Copy link
Contributor

rsc commented Aug 17, 2022

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc moved this from Incoming to Active in Proposals Aug 17, 2022
@rsc
Copy link
Contributor

rsc commented Sep 21, 2022

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

@rsc rsc moved this from Active to Likely Decline in Proposals Sep 21, 2022
@database64128
Copy link
Contributor

Is this still something people need?

Yes. Some protocols put the authentication tag before the ciphertext. Having this new API could help simplify the implementation of such protocols.

@rsc
Copy link
Contributor

rsc commented Sep 28, 2022

Which protocols do that? What is the API you would need for those?

@rsc
Copy link
Contributor

rsc commented Sep 28, 2022

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc moved this from Likely Decline to Active in Proposals Sep 28, 2022
@rsc
Copy link
Contributor

rsc commented Oct 5, 2022

/cc @golang/security

@FiloSottile
Copy link
Contributor

I don't remember why I was interested in this in 2018, but I know I haven't needed it since.

Note that this is exclusively a performance improvement: you can already allocate a slice, put the ciphertext||MAC output in there, and copy them separately.

Given how little this is used, and how there's a workaround for those use cases, I don't think we should do this.

@jyxjjj
Copy link

jyxjjj commented Oct 6, 2022

I don't remember why I was interested in this in 2018, but I know I haven't needed it since.

Note that this is exclusively a performance improvement: you can already allocate a slice, put the ciphertext||MAC output in there, and copy them separately.

Given how little this is used, and how there's a workaround for those use cases, I don't think we should do this.

I don't remember too, and also haven't need it since.

But I don't think we don't need it. Even we have other way to get them as we want.

To copy them separately will be more like C behavior. Will be more easy to Integrate with other languages.

Be more friendly to beginners.

@rsc rsc moved this from Active to Likely Decline in Proposals Oct 12, 2022
@rsc
Copy link
Contributor

rsc commented Oct 12, 2022

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

@rsc rsc moved this from Likely Decline to Declined in Proposals Oct 20, 2022
@rsc
Copy link
Contributor

rsc commented Oct 20, 2022

No change in consensus, so declined.
— rsc for the proposal review group

@rsc rsc closed this as completed Oct 20, 2022
@golang golang locked and limited conversation to collaborators Oct 20, 2023
@rsc rsc removed this from Proposals Oct 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
None yet
Milestone
Proposal
Development

No branches or pull requests
7 participants
@riobard @rsc @FiloSottile @agnivade @gopherbot @jyxjjj @database64128