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: crypto/cipher: ability to use AEAD mode as block mode #23514

Closed
creker opened this issue Jan 23, 2018 · 16 comments
Closed

proposal: crypto/cipher: ability to use AEAD mode as block mode #23514

creker opened this issue Jan 23, 2018 · 16 comments
Labels
FrozenDueToAge Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone

Comments

@creker
Copy link

creker commented Jan 23, 2018

Currently AEAD interface provides only two functions

Seal(dst, nonce, plaintext, additionalData []byte) []byte
Open(dst, nonce, ciphertext, additionalData []byte) ([]byte, error)

That works for network protocols where messages are usually small. That may work for small files but that doesn't work for larger payloads. GCM provides enough security to use its tag to authenticate large payloads that you wouldn't want to (or couldn't, even) store in memory in its entirety. With the current implementation there's no other way.

I propose to change existing AEAD interface or add another one to give the ability to encrypt/decrypt payload block by block. When finished it would produce authentication tag that you may use as you see fit. The tag could be stored at the end of the cipher text implicitly but I don't really like the idea of implementation forcing me to something that isn't required.

All implementations that I'm familiar with and has used myself divide into three steps:

  1. initialization where you set key, nonce and AAD
  2. update where encryption/decryption happens
  3. finalization

The main difference is in finalization:

  • JCA and NetFramework use it to output leftover plain text or leftover cipher text with the tag attached at the end. Tag verification is done implicitly.
  • OpenSSL use it to output leftover cipher/plain text and verify tag. The tag is accessible through a separate routine on encryption and should be provided by the programmer on decryption.
  • mbedtls does everything in update and uses finalization only to return the tag. Tag verification is done by the programmer.

All of these implementations fit the proposal. My only preference is that the tag should be separate from the payload. The implementation shouldn't write or read it implicitly. Programmer should have the access and freedom to use it as needs to. OpenSSL and mbedtls both implement it that way.

@gopherbot gopherbot added this to the Proposal milestone Jan 23, 2018
@dgryski
Copy link
Contributor

dgryski commented Jan 23, 2018

/cc @agl

@balasanjay
Copy link
Contributor

agl's previously written about this: https://www.imperialviolet.org/2014/06/27/streamingencryption.html.

@danphamus304
Copy link

AEAD

@danphamus304
Copy link

Seal(dst, nonce, plaintext, additionalData []byte) []byte
Seal(dst, nonce, plaintext, additionalData []byte) []byte
Open(dst, nonce, ciphertext, additionalData []byte) ([]byte, error)
Open(dst, nonce, ciphertext, additionalData []byte) ([]byte, error)

@creker
Copy link
Author

creker commented Jan 23, 2018

@balasanjay I don't think small propability of people misusing API is a justification to artificial limitations. There's always a chance of misuse with crypto. In fact, there's a much bigger chance with nonce provided by the developer, given that GCM is very strict about choosing it. The reasoning behing API design looks more like a personal opinion without clear justification (and I don't think there could be any). The blog post pretty much says that.

With current API you have no other choice but to ditch standard library and use external library to have access to something that should be there in the first place. That doesn't look good for standart library and Go 2 is a good oportunity to fix that. Everything else in the crypto library looks good and does everything you need it to do. AEAD interface always stood out for me as being the "special one" for no reason.

@creker
Copy link
Author

creker commented Jan 23, 2018

And I don't propose to remove Seal and Open. They're handy and easy to use. Documentation can even say that programmer should use them in every possible case and resort to block mode of operation only if he really needs to.

@balasanjay
Copy link
Contributor

Just to be clear, I wasn't actually saying anything myself, I was just pointing out that the actual decider (agl) has written about this matter previously, and included a sketch of a design with 16kb chunking and multiple implicit tags to avoid adding modern crypto APIs that return unauthenticated plaintext.

@agl
Copy link
Contributor

agl commented Jan 25, 2018

A scatter/gather interface (which allows one-shot operation, but where the expansion due to encryption can be written/read from a different buffer) is a reasonable extension to the AEAD interface that Go doesn't currently have. That allows, for example, in-place encryption while storing the expansion data (i.e. the tag in AES-GCM) into a separate buffer.

(For example, see this API.)

But a fully general interface is dangerous for the reasons referenced above and we don't want that. Code can always do it by avoiding the standard library, but it's ok if the standard library is constraining when you're going off the rails.

@creker
Copy link
Author

creker commented Jan 25, 2018

I understand the danger. I just don't see it to be enough to constraint the standard library in such a way that it's incompatible with other implementations. This is the only general crypto library where I've seen this done without any alternative.

Look at things like MD5 and DES. Going by the same logic, they shouldn't be in the library. They're not only dangerous but completely broken at this point. But there're implemented and have a comment saying that you shouldn't use them. And I don't see the problem here. Even though there're broken, it's still necessary to provide them for compatibility with other software. The same reasoning applies here - AEAD could be misused but you can always mention that in the documentation. Keep the existing interface for safe and easy tasks and provide another for users who know what they're doing.

As for scatter/gather, that's not the point of my proposal but it's at least something if fully general interface is rejected.

@aead
Copy link
Contributor

aead commented Jan 27, 2018

@creker

I understand the danger. I just don't see it to be enough to constraint the standard library in such a way that it's incompatible with other implementations. This is the only general crypto library where I've seen this done without any alternative.

If you look for example at the AWS SDKs you can see that they have the same problem described in the issue. Their SDKs let the user process data before the auth. tag is verified. In fact the only SDK which avoids this pitfall most users are not aware of is the go-sdk. (as far as I know)

Look at things like MD5 and DES. Going by the same logic, they shouldn't be in the library. They're not only dangerous but completely broken at this point.

These algorithms are there for legacy and backward-compatibility reasons. Removing them would be a breaking change. Actually they should have been implemented in x/crypto - not in the standard library. Roughly speaking: "One mistake does not justify another".

Anyway as mention in Adam's blog post it would only be a valid approach for very few use-cases. IMO it's not the responsibility of the standard library to provide a solution for any use case but it should focus on the common and recommended cases.
If you're trying to use an AEAD in streaming mode you should consider an approach like sio

@creker
Copy link
Author

creker commented Jan 27, 2018

@aead

In fact the only SDK which avoids this pitfall most users are not aware of is the go-sdk. (as far as I know)

I see that and don't view it as an advantage. It's not a kids playground, it's a crypto library. I don't think one should constraint API to such an extent just so that some programmers that don't understand how authenticated ciphers work (like, really don't understand. That's basic stuff) do not make a mistake. If that's the goal here then you really shouldn't be able to choose nonce yourself. That's even more subtle and there're an actual recommendation from NIST for that, unlike the issue here.

One mistake does not justify another

As I said in my post, I don't see it as a mistake. There's a reason for it - compatibility. Being able to use it to communicate with other software. Maybe not exactly like it but the issue is similar to that. Like, you have already implemented protocol that uses GCM for perfectly secure encryption of large payloads but suddenly Go says "no, you're not allowed to do that because somebody somewhere might make a dumb mistake. Go use external C library and mess your project by introducing CGO into it".

the common and recommended cases.

File encryption is common case for AES GCM and I don't see anywhere where GCM is explicitly recommended to be used for small payloads. Again, that looks like a personal opinion to me that for some reason got into general crypto library. There was a mention of BoringSSL - that's exactly where that kind of constraint should be. In application specific internal library and BoringSSL is exactly that.

@aead
Copy link
Contributor

aead commented Jan 27, 2018

I see that and don't view it as an advantage. It's not a kids playground, it's a crypto library. I don't think one should constraint API to such an extent just so that some programmers that don't understand how authenticated ciphers work (like, really don't understand. That's basic stuff) do not make a mistake.

About the AWS example: Users use the SDK API without choosing a nonce - the SDK does that (and users may also not be aware of the underlying cipher at all). It's really a high level API. Nevertheless the "bad" practice of returning unauthenticated data shines through the API and may hurt users which just use a high-level API and are not familiar with cryptography at all.

If that's the goal here then you really shouldn't be able to choose nonce yourself. That's even more subtle and there're an actual recommendation from NIST for that, unlike the issue here.

That's a conceptual requirement. If the nonce would be chosen randomly by e.g. rand.Reader you would not be able to decrypt. I see that there is the possibility of misuse but I don't see a proper solution for this.

There's a reason for it - compatibility.

Well that's a tradeoff. API flexibility vs. misuse resistance. Since I see only very few legitimate uses cases of an AEAD streaming approach I think misuse resistance outweigh API flexibility in this case.
I don't think it's an opinion - it's a design choice.

File encryption is common case for AES GCM and I don't see anywhere where GCM is explicitly recommended to be used for small payloads

As I tried to show with the AWS example it's not that simple. AWS CSE is basically file encryption (objects are just blobs - like files) - and they implemented it in - at least - a dangerous way. As recommended before you can split files into smaller chunks and encrypted them separately instead of the whole file. Further there's the 64 GB bound for AES-GCM - what if files are larger than 64 GB?

Again, I don't see why it's Go's responsibility to support every use case - especially if support exposes an API which leads to bad practice - even tough there are a few legitimate cases.

Anyway I cannot make any decision about this - That's @agl 's responsibility.

@rsc rsc changed the title Proposal: crypto/cipher, add ability to use AEAD mode as block mode proposal: crypto/cipher: ability to use AEAD mode as block mode Jan 29, 2018
@rsc
Copy link
Contributor

rsc commented Jan 29, 2018

/cc @FiloSottile

@FiloSottile
Copy link
Contributor

As a standard library for developers, not cryptographers, I think we have a clear and well-justified (if not perfectly implemented) policy of preventing mistakes that have been commonplace in other ecosystems. I don't think that's up for discussion.

However of course compatibility involves a constant tradeoff, so I'd like to understand how common these use cases are. You mentioned some protocols securely using GCM over large payloads, can you make the example(s) more specific?

About scatter/gather, not opposed, but I'm not sure I see the application well. Most times in Go I see buffers with capacity including Overhead being reused/pooled, and sliced down when not holding ciphertext.

@ianlancetaylor ianlancetaylor added the Proposal-Crypto Proposal related to crypto packages or other security issues label Mar 20, 2018
@rsc
Copy link
Contributor

rsc commented Jan 15, 2020

Sorry for leaving this for so long, but based on the discussion above this sounds like a likely decline.

Leaving open for a week for final comments.

@rsc
Copy link
Contributor

rsc commented Jan 22, 2020

No change in consensus, so declined.

@rsc rsc closed this as completed Jan 22, 2020
@rsc rsc moved this from Likely Decline to Declined in Proposals (old) Jan 22, 2020
@golang golang locked and limited conversation to collaborators Jan 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
No open projects
Development

No branches or pull requests

10 participants