-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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 block cipher modes of operation to std.crypto #5763
Comments
What is the use case for these in the standard library? |
From my reply on my IGE PR: As I see it, if the goal of Zig is to be able to stand independent of C, it makes sense to have as strong a crypto implementation as possible. Right now the AES implementation is really lackluster and missing a lot of the block ciphers which are used pretty commonly in many different applications, and the one cipher that is included, CTR, is even missing a way to decrypt the encrypted payload. Even if the crypto stuff eventually gets moved to its own library, independent of std, I think it's important to have strong cryptographic implementations available. |
I agree it makes sense to have a strong crypto implementation in the standard library. However, Zig can stand independent of C regardless of whether it supports modes such as ECB and CBC (neither does C actually, most people use OpenSSL et al). I would argue further that modes such as ECB and CBC are not only not strong but dangerously broken and should be left out explicitly from the standard library as a matter of safe design. Without even going into ECB, there are just too many reasons to stay away from CBC: https://blog.cloudflare.com/padding-oracles-and-the-decline-of-cbc-mode-ciphersuites/ I cannot see that adding ECB and CBC into Zig's crypto would make it strong, and I think the friction added to projects that need those modes is only a good thing. |
CBC is the most ubiquitous block cipher mode in use today. Much of cryptography has nothing to do with SSL and HTTPS, and outside that space, CBC is king. While there are important padding attacks against CBC, CTR has much more catastrophic failure modes and should never be used without without extreme caution. Most people get CBC wrong by using fixed IV, leading to problems in the first block (or occasionally first several blocks depending on the structure). But most people get CTR wrong in exactly the same way (using a constant nonce) and that leaks the entire key. CTR is incredibly dangerous to offer as the only available mode. Few non-experts understand how to use it correctly. A correct CBC implementation should absolutely be included in the standard library in order to provide interoperability with common data formats from other languages. Otherwise developers are forced to write their own CBC implementation for interoperability, which is exactly the wrong direction. If anything is to be removed here, it's CTR, which is both relatively rare and very dangerous (even if a perfectly fine mode when used correctly). If only one mode were offered it should be GCM. But CBC should be second in line, followed by ECB in an explicit |
By the way, just so people have an understanding of the plans here: I personally know enough about crypto to use it responsibly, but I'm not actually qualified to audit crypto code, or even to know enough to organize the std lib APIs in a way to encourage best use. For now I've been accepting crypto code into the std lib when it is well tested and the code is clean enough. The plan is to spend some of the Zig Software Foundation funds before releasing 1.0 to contract a security expert to audit the std lib crypto code and consult on the API design to encourage best practices (and figure out which APIs to strategically omit from the std lib). |
Understood. I'm currently exploring this space while I try to implement the RNCryptor format in Zig. I'm almost done with a PR to add PBKDF2 to the stdlib. The current crypto code makes me a bit nervous because it doesn't really feel designed for crypto uses. It doesn't separate block modes from block ciphers, for example, and breaking out stream ciphers, but not block ciphers feels very ad hoc. It might be too early to be putting these kinds of very touchy things into the stdlib. Definitely agreed that some things should probably be omitted. But I've reviewed enough ad hoc security code to know that just leaving too much out begs for it to be added back badly. |
That's good to know. I would consider rectifying this a release blocker for 1.0. |
I disagree. The "very dangerous unique nonce" argument against CTR would apply equally to AEADs, except CTR accepts a larger nonce than most AEADs so it's actually easier to get right with a random nonce (except for XChaCha20-Poly1305). Used correctly, with a unique nonce, CTR is a perfectly sound cryptographic primitive in an encrypt-then-mac construction. On the other hand, beyond simple misuse, CBC has too many known attacks. Looking to the future of encryption at rest and in transit, I don't think CBC has a place in the std lib but we can agree to disagree. But there's absolutely no reason for ECB to be in Zig's std lib, even in an "unsafe namespace". "Electronic Code Book" mode is completely broken. This is what ECB does to Tux: |
Always a great picture. I include it in all my crypto talks when explaining modes. ECB is not broken. It's a perfectly useful mode if used correctly (just like CTR is excellent if used correctly, but often is not). ECB has fewer correct uses than CTR, but I personally wind up using it a lot because I work on embedded systems that transmit very short messages over Bluetooth, and a 16 byte IV or a counter-synchronization scheme is impossible over BLE advertising. I'm actually building a new ECB-based encryption scheme this morning, and it's secure because the entire payload is less than 16 bytes and every payload is unique. I've also used it securely in key-wrapping protocols (since the keys are random and never repeat). I agree it's tricky to use correctly, but if you know what you're doing it's an important tool. This is not an argument for putting it in std.crypto. It's just that it shouldn't be banned for being insecure. MD5, on the other hand, is not possible to secure, but is part of std.crypto. It's a very useful hash. It's just not a cryptographic hash anymore. It really is completely broken. My question here for @andrewrk is what the goal is for std and std.crypto. Right now, IMO, it's a hodgepodge, which makes total sense, but I think can be improved. I see the following as reasonable directions:
There are many middle-grounds here. There could be something like the "/x/" namespace in Go that collects things that are useful enough to be officially managed, but not useful enough to be in std. There could be an "Insecure" namespace like Apple's CryptoKi's to shun things that are useful but not recommended (Apple puts MD5 and SHA1 here). But I think the first question is what the overall goals for std and std.crypto are. |
I think right now we essentially have the kitchen sink (for the entire std lib), because it serves as a test bed for the language. However as Zig matures it will change directions to be much more minimalist. I expect there to be a great purging of APIs from std lib once we have the package manager working well. As for crypto, the direction I see the std lib going is something close to the Go approach, however possibly more minimal, with more tasks out-of-scope that people are expected to solve with third party libraries. The std lib will have the necessary components to make the package manager work, since that has to ship with the compiler, which drags in a lot of networking stuff. So in that regard we would be similar to the Go approach. In some ways a language, compiler, and std lib is never finished because new CPU processors come out with new instructions to support, operating systems change their APIs so that cross platform abstractions have to be modified, and in the case of crypto, which algorithms are the recommended ones to use for various use cases changes over time. However I do want to reduce the number of items causing the language, compiler, and std lib to change over time. I want them to be "done" as much as possible. So I do want to push that maintenance burden away from std and into third party packages as much as possible. |
That all sounds very good. If you need TLS to be "in the language," that's going to bring in quite a bit of crypto, but I think it's reasonable to kind of draw the line around that and say almost everything else is outside. But if it's kitchen sink for now, I have some algorithms I'll make PRs for so I can build a zig implementation of my crypto format. One thing I'd like to explore is splitting the cipher from the mode. Currently there's an "AES" and it has a ".ctr" function. Instead, I expect there should be either a CTR that takes an AES, or an Encryptor that takes both a mode and a cipher. I'll play around with this and try to make a more concrete proposal in the form of a PR. (I've really enjoyed playing around with zig so far. Great work.) |
I was surprised to find while digging through the standard library code today that the crypto algorithms are completely implemented in Zig. Good job to those who did so! Something I noticed was missing, though, is AES block cipher modes such as ECB, CBC, OFB, CFB, and IGE. It would be really nice if we could get these integrated as well.
I don't imagine most of them would be too hard since they just build on top of AES.
Progress:
The text was updated successfully, but these errors were encountered: