-
Notifications
You must be signed in to change notification settings - Fork 18k
proposal: x/crypto: deprecate unused, legacy and problematic packages #30141
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
Comments
I would prefer to see Additionally, I performed a quick survey via godoc.org importers and it shows that the |
@davecgh I am not aware of any reasons to pick RIPEMD-160 for a new design, so the fact that cryptocurrencies inherited it from the Bitcoin paper is not a good enough reason not to deprecate it. I don't expect it will require any maintenance anyway—it's a hash function, the package is almost certainly fine as is—but deprecating it will send the message to users not to look there if they just need a hash function, and communicates that we would for example not take an optimized assembly implementation for it. |
Just picking two examples to illustrate my opinion: Both are dated, one is a PITA. Yet I think x/crypto should not be reduced to state of the art ciphers/protocols since we are not always developing self contained systems where we can safely choose the best available tech, we also have to connect to legacy systems or simply not-that-well-developed third-party systems. Also I think that especially the pretty self contained and matured algorithms like MD4 don't suffer much from "being unmaintained". I mean: what would you maintain? The algorithm is stable (although "broken"). We cannot do anything about attacks on the algorithm (apart from refraining from future use). I understand your point with OpenPGP. Though it might also be an option to try and get one of the forks to be reintegrated and actually maintain that. If a living protocol (that still receives updates; has a moving spec) rots ... yeah, throw it out. The sophisticated implementations however, that are simply not "maintained" because there is nothing to maintain there ... I would like to keep them. |
The reasons given for obsoleting the Aside from this argument I admittedly have nothing else with which to claim that keeping |
|
I'd rather keep openpgp (and would consider making it compatible with GnuPg2 ). Still the problems there are quite surprising Would be cool to have someone to take a look. |
I'd similarly be very disappointed to see openpgp removed. Frankly, the implementation even as it exists is much higher-quality than the upstream GnuPG one, incomplete as it may be; removing it would thus be a loss to the community as a whole. Admittedly, I'm one of those folks with a private fork, but I have approval from my employer's legal team to submit our changes upstream; if getting off my arse and doing so is the cost of keeping it in-tree (and thus being sure that the existing functionality continues to work with future versions of Go), I'm happy to do so. |
@tredoe @charles-dyfis-net, this issue is about freezing+deprecating them, not removing them. |
@bradfitz To be fair, deprecating them has almost the same effect as removing them; people will look elsewhere for alternatives that aren't marked as deprecated. Which I imagine is the intended effect. Unfortunately, the alternatives may not always be of higher quality. For example, I'd much rather see people use x/crypto/md4 over some other, random md4 implementation. I don't think the "these algorithms are broken" holds as an argument. A lot of legacy still relies on them, unfortunately. As such, we should rather look at the cost of not deprecating a package. One can of course argue that another well-organized party that isn't Go should maintain legacy packages. Have we explored why these packages made it into x/crypto in the first place? |
I disagree. The effects are quite different: removing them breaks users, and deprecating them does not. Deprecating them is just adding a comment saying, "Yo, new applications shouldn't use this, and also we're not adding new features so don't send PRs."
Sounds like the intended effect is reducing maintenance cost by not accepting changes to add features or optimize the performance. |
I think the "deprecated" marker would move the same thinking you have here down the line. Your argument for deprecation (or removal) is that the code is old and the algorithms should not be used. While the code is actually most likely still fine and just doesn't need any maintenance (if it does, someone will report it and or create a PR). |
Case in point: dominikh/go-tools#413 Everywhere in the standard library, deprecated means "stop using this, move to an alternative". |
Remember that deprecation comes with a message, so we can explain the reasons and implications for each package individually, and I'll make sure to provide as much context as possible. There will be packages that say "this algorithm is broken [or superfluous] because X and should not be used except for compatibility with legacy systems". This does not call for an alternative implementation, but it warns people off using it, and recommends moving to non-broken things. Other packages will say "this implementation is incomplete and unmantained, consider an alternative protocol like X, or a community maintained package". This hopefully leads to a centralized target for maintenance efforts which does not involve our resources. I am replying to a few points individually below, before commenting please check if your point was already made above, to prevent this from becoming a giant thread nobody reads or feels like participating in.
Indeed, deprecating them comes at near-zero cost because there are not going to be new features and bug fixes. But the message to developers about not using them is important.
Ack, but I'm afraid that's not enough to justify the Go project maintaining an implementation. I would probably not even accept a Signal protocol implementation in x/crypto.
Good point! That makes it fundamentally vulnerable to Sweet32, updated the proposal.
I use usage in public projects as an indicator of popularity and therefore usefulness to the ecosystem, not as proof that nobody uses the package at all, and that's one part of the tradeoff decision on whether it's worth the resources to maintain.
AES-256 is secure enough for Top Secret data in NSA's Suite B, I don't feel the need to carry anything else to fill a security gap.
XTS is quite bad in fact. See also a better tweakable cipher.
Almost half of all issues open against x/crypto packages are for x/crypto/openpgp, and a good chunk of all CLs are for it as well. I'm not blaming the community for not contributing back here, I blame the protocol for requiring too much maintenance to properly implement. It just doesn't make sense for us to put in the effort to do the review and design work to bring this package to a state in which we are happy about it. Hopefully the community will centralize on a fork.
MD4 is a bad example, because it's literally as broken as a hash function can be. Go is a memory-safe language, so a random implementation off the street can't introduce an RCE without really trying, and the next worse thing a hash function from the last 20 years can have is a collision, and well, I might still have some Python to generate one on a laptop.
The cost includes developers being misled into thinking that a primitive or protocol can serve their needs because it's in x/crypto, diluting the safety properties of using the standard library.
Some to support OpenPGP, some have always been there from before there was a conscious effort to limit what was in x/crypto, some were reasonable at the time and aren't anymore, some were a mistake.
If you are using any of the above, I would like you to stop using it and move to an alternative. Sometimes an alternative implementation, sometimes an alternative algorithm. If you know you can't because of legacy compatibility reasons, at least I made you aware of it, and made sure the question got asked at review time, and the answer hopefully documented. |
Having seen cryptography mismanaged in the standard libraries of many other languages I use, this seems like a great move to me, and a breath of fresh air by comparison. Cryptography in the standard library should be held to a high bar, and I'm glad to see that bar enforced. In languages with a "batteries included" standard library like Go's, the ecosystem is largely shaped by what's available in the standard library. Go has pretty much the best cryptography ever seen in a language's standard library with a few exceptions, and this proposal covers most of the exceptions. For implementations of legacy protocols, I think users are almost certainly better off with something out-of-tree. This not only discourages accidental use in new projects, but also provides greater agility in the event of a security incident, as the response does not involve a new release of the language/standard library. Deprecating these algorithms from the standard library isn't a value judgment on their overall utility, just a signal that people should move to out-of-tree implementations. The remaining concern there is that they will lose the reluctant maintainers these algorithm implementations previously had. If the algorithms are truly useful and important, I think you can find new maintainers, and ones who aren't reluctant. Otherwise, it's entirely possible they're not as useful as they're claimed to be. The remaining arguments seem to fall into a category I call "unusual requirements". For things like symmetric ciphers and hash functions, there are huge reasons to prefer algorithms like AES and SHA-2 over things like Serpent, Twofish, or RIPEMD-160, namely AES has hardware acceleration through a number of different platform-specific instructions like AES-NI which provide better performance and sidechannel resistance. In the case of RIPEMD-160 it has only 80-bits preimage resistance which is considered rather weak by today's standards. The specific security and performance requirements (much less threat model) weren't stated in any of these cases, but in my opinion as a cryptography professional specializing in symmetric encryption I think the claims made by people alleging they selected these algorithms due to "unusual requirements" are, to put it bluntly, factually inaccurate. tl;dr: eliminating junk cryptography from the Go stdlib will help improve the overall security of the ecosystem, so I say go for it. |
@tarcieri, this bug is about x/crypto, which is not the standard library. |
@bradfitz well don’t I look foolish. Mea culpa |
Hey! Sorry for jumping into the discussion.
Well, with the upcoming OTRv4 protocol, I don't how true this will still hold. There is a lot of people now interested in using OTRv4 again and implementing it on different clients; so I don't if it will be a shame to loose it here. That said, for the most people who are actually using OTR in golang, they actually use this library instead: https://github.com/coyim/otr3 I also don't know the state of OTR in golang (if it is fully implemented). I can check if needed. I don't know what MLP means, is that MLS? Btw, here some useful links around OTRv4, in case someone wants to check its interest/proposal:
Hope this information helps :) |
I'm happy to hear there is work on the protocol, and that there is a community package for the previous version. That suggests deprecating the x/crypto one would be good for the ecosystem, letting it focus on modern and maintained implementations.
Yes, thanks, fixed. |
Simply because the attack is possible doesn't mean it is particularly easy to carry out. For Sweet32, the attacker needs to be able to monitor traffic passing between the user and a vulnerable website, as well as control JavaScript on a web page loaded by the user's browser. It would take about 38 hours to collect hundreds of gigabytes of data necessary to decrypt the authentication cookie. https://blog.securityevaluators.com/what-is-sweet32-4a62dca90296
Fair point
Although NSA has categorized this in Suite B, they have also recommended using higher than 128-bit keys for encryption. So, should be also removed or deprecated AES-128? Besides of that, and althought I wouldn't assume that NSA has cracked AES ciphers, I would assume that most crypto systems that use AES have implementation flaws that the NSA could exploit when they want throught its own data center, because the NSA are actively subverting security on the standard level or in collusion with software developers.
At the first, in practice, many encrypted filesystems already use encryption that's strictly worse than XTS, and they generally use the same block-based model as full disk encryption for exactly the same performance reasons. Besides, the title of such article is confusing because many people will only ever encounter XTS when setting up full disk encryption with dm-crypt, where they'll be presented with the choice between XTS and CBC-ESSIV. And finally, it's also not saying that you shouldn't use XTS for full disk encryption. In fact it seems to say it's probably OK for full disk encryption: "It’s certainly better than ECB, CBC, and CTR for FDE. For the crappy job we ask it to do, XTS is probably up to the task." |
I would agree that getting rid of otr(which I believe is v2?) from x/crypto makes sense, there's an otrv3 implementation(https://github.com/coyim/otr3) in Golang and a partial implementation of otrv4 in Golang(https://github.com/otrv4/otr4) underway. As we've been discussing internally in some calls with Ian Goldberg, we'd like to get rid of v2 in favor of v3 and especially v4, so, keeping the otrv2 implementation in x/crypto doesn't make much sense. |
Yes, I agree with @FiloSottile and @DrWhax . We will surely develop OTRv4 in golang, and it will maintained; so deprecating the old version in golang libs (which might be in between v2 and v3) makes sense. In the future, if wanted, we can implement OTRv4 in golang libs itself. Thanks! |
I can confirm that well-meaning but uninformed developers choose equally randomly among the options in Deprecating an algorithm in x/crypto isn't the same as banishing it from the Earth, it just means Go's very few crypto reviewers will have more time to focus on things that are/should be widely used. |
This proposal should be put on hold for now. We shouldn't remove anything from x/ until (1) modules are enabled by default and (2) we have started tagging versions. Once we have started tagging versions, we can discuss removing things; modules will mean users don't automatically get the latest copy, and the tagged version will make it easy to record that what exactly they depend on. |
@rsc perhaps I'm missing something, but it appears that removal isn't part of the proposed plan here:
|
Taking md4 as an example, obviously that should be marked as completely broken and something no one should use except under extreme duress. It's funny that crypto/md5 says
but md4 does not. Obviously it should. Adding that kind of documentation note doesn't require a proposal. Similarly, x/net/websocket says:
That's another very clear message that seems fine. But it seems like a few are not quite in that camp. OpenPGP for example, isn't cryptographically broken, right? I'm less sure about marking that package as deprecated. Is there an established fork that we should point to instead? Or should we invite them to own x/crypto/openpgp? |
@rsc I want to send a stronger message than just adding a line to some package-level docs that nobody will pay attention to (at the very least for existing dependencies). I want developers to make an active, hopefully justified, choice to ignore a tooling-provided flag. If you need some WebSocket feature you'll know, and if it's not in golang.org/x/net/websocket, you might look at the docs and learn where to go look for it. If you are using a bad hash, it will look and feel like a perfectly good hash.
OpenPGP is legacy, brittle, and very high maintenance to implement. I discourage any new system from using it, and I don't think it's worth any of our limited resources to maintain. If it were only for lack of resources, I would be seeking an owner for x/crypto/openpgp, but given that it's not something people should be encouraged to use, I don't think that would be a service to our users. I expand on this in the "Why" section above, under the "openpgp/" bullet, and at #30141 (comment). |
@Merovius As I understand it, you want a 3rd-party maintained I think both are valid options, backed by a number of arguments. Like many decisions, this is about a trade-off, and I stand by my opinion on what is best for the ecosystem. |
@FiloSottile Sure. But FTR, there is a third option, which is that the Go team stays responsible (and note that "being responsible for" isn't the same as "doing all the work") for it - just that it's not you specifically, if you don't want to do it. It'd my preferred option, which is why I'm making it explicit. So far it sounds as if you're categorically ruling out the existence of a package in |
As a user of OpenPGP, I'm happy to host the |
(Was asked to comment here.) I see three broad categories of problematic packages here: First are the simple packages that are just bad ideas: md4, cast5, etc. Here the issue is that inclusion in x/crypto may convey an inappropriate (and unintended) blessing, but they're basically zero maintenance. (BoringSSL calls them “decrepit”.) Second are the barely used packages: xts and xtea, for example. Here the issue is that these packages are so obscure that they don't meet the bar for inclusion. This is my fault: there was a time when we (or at least I) were significantly more promiscuous in accepting things. These are also basically zero maintenance (because nobody cares) and there is a little overlap with the first category too. (For example, using twofish doesn't make sense now.) Third, and most thorny, are the “Adam used to use x/crypto as a personal repository” packages: openpgp and otr being the obvious examples. openpgp came about because Brad asked me years ago for something to do encryption-at-rest in Camlistore and OpenPGP was a) a standard, b) not terrible, and c) interoperable with other stuff, which seemed like it might be useful. But OpenPGP is a large standard and over the years some people have tried to extend the package to cover more modern additions. Several of them tried to rewrite large parts (poorly) and I declined the changes. More recently, I've been affirmatively ignoring Go stuff because Filippo exists. But the package should probably have lived in Camlistore. otr exists because I had a friend who wanted to use OTR over XMPP and, after taking a look at the code quality of some XMPP clients, I decided that I should write my own. The OTR part was landed in x/crypto because it seemed relatively independent and, recall, x/crypto had a rather lower bar because Go usage was dramatically smaller back then. But these packages are big and really need an active maintainer. Also, I suspect basically nobody is looking to find the horrendous security bugs that I've almost certainly botched in there. But being in x/crypto means that we have to maintain a high quality, but Filippo & I are not putting in the time to review and maintain them, so they're deadlocked. bn256 is special in that it falls into all three categories!
For the first group, one cannot escape that interoperability does mean that, sometimes, they cannot be avoided. But I agree with Filippo that a warning in the package documentation seems insufficient and that including them at the top-level of x/crypto seems wrong now. Freezing and deprecating them seems like a minimal step. For the second group, we should be on a path to removing them at some point and their obscurity hopefully means that the impact will be tiny. As a first step, freezing and deprecating seems fine to me. The third group is the most nettlesome. The OpenPGP spec is problematic in several ways but there's not much else in the space and at least it's (much) better than CMS. However, they're already effectively frozen in the environment of x/crypto because our standards are too high and we're not willing to put the effort into maintaining them. I think they should move elsewhere: hopefully each to a single canonical location. I don't have a detailed prescription, but I do feel that their future does not lie in x/crypto and it would be good to establish that officially. |
We've created a golang-openpgp mailing list: https://groups.google.com/group/golang-openpgp Please join if you're an interested golang.org/x/crypto/openpgp user, contributor, or possibly as a maintainer. (Our goal is broaden its maintainership.) |
We will be deprecating the packages listed above, except I'll update the proposal and send a CL shortly. |
Hey! Thanks @agl and @FiloSottile for all the very nice explanations. Around OTR, as said, if you are willing, as the person right now leading the OTR project, go ahead deprecating it. The current code that is in the golang repo seems like a mix between v3 and v2, and we have decided with Ian Goldberg to completely deprecate "globally" v2 (https://bugs.otr.im/lib/libotr/issues/140). That said, we can also try to improve the current state of OTR in the golang library, but we will prefer to maintain our own library. I see that now there is a 'golang-openpgp mailing list', will there be on for OTR? At the end of the day is your call :) But we will not like users using a very old version of OTR. Thanks! |
We don't plan to make a golang-otr mailing list, but you're welcome to if you think it'd be useful. |
Change https://golang.org/cl/163537 mentions this issue: |
Thanks @bradfitz ! I don't think it will be super useful :) Looking forward to the open-gpg list content. |
That's basically not true. OTR support has been withdrawn from major clients such as Conversations and Gajim as OMEMO is widely supported. As for OTRv4 no major client plans to implement it, I am aware of only one implementation in the wild (Coy.im, by authors of OTRv4). |
Hey @wiktor-k ! Thanks for your comments here; but as @FiloSottile has widely said around OpenGPG, I don't think this is the place to litigate the state of something. That said:
I don't think that having been removed from two clients is bad in the OTR world (one by the authors of OMEMO). And, for OTRv4, I can tell you that a lot of clients (not only related to XMPP, as OMEMO is only for XMPP) are planning to implement it. :) Thanks! |
I did comment only on the OTR being standard in XMPP world being grossly untrue. Even XMPP Wiki says that "OTR has widely been replaced by OMEMO in the XMPP network and is recommended to be used instead."
You may want to list these clients on OTR clients page. The current list that contains unmaintained projects (such as Adium or Chatsecure) doesn't inspire much confidence. |
Sure. But, as I read that, and correct me if I'm wrong, that seems to be a recommendation.
OTRv4 protocol design is still in development. Listing on a page which ones will support it seems dishonest and advertising-like. Once everything is done, we can put them here. So worry not. :) |
It seems to be some ambiguous wording out there but the history page says "recommend OMEMO as replacement for OTR". I'll get this sorted out in XSF. Thanks for bringing it!
Good luck and godspeed 👍 Now we can finally put this thread to rest... |
Fixes golang/go#30141 Change-Id: I76f8eae31cfd6d106440114685cc0d9abba374f8 Reviewed-on: https://go-review.googlesource.com/c/163537 Reviewed-by: Adam Langley <agl@golang.org>
Fixes golang/go#30141 Change-Id: I76f8eae31cfd6d106440114685cc0d9abba374f8 Reviewed-on: https://go-review.googlesource.com/c/163537 Reviewed-by: Adam Langley <agl@golang.org>
Fixes golang/go#30141 Change-Id: I76f8eae31cfd6d106440114685cc0d9abba374f8 Reviewed-on: https://go-review.googlesource.com/c/163537 Reviewed-by: Adam Langley <agl@golang.org>
Fixes golang/go#30141 Change-Id: I76f8eae31cfd6d106440114685cc0d9abba374f8 Reviewed-on: https://go-review.googlesource.com/c/163537 Reviewed-by: Adam Langley <agl@golang.org>
Fixes golang/go#30141 Change-Id: I76f8eae31cfd6d106440114685cc0d9abba374f8 Reviewed-on: https://go-review.googlesource.com/c/163537 Reviewed-by: Adam Langley <agl@golang.org>
Fixes golang/go#30141 Change-Id: I76f8eae31cfd6d106440114685cc0d9abba374f8 Reviewed-on: https://go-review.googlesource.com/c/163537 Reviewed-by: Adam Langley <agl@golang.org>
x/crypto is meant to follow the same philosophy as the rest of the Go cryptography standard libraries: safe, useful subsets of widely used and robust primitives and protocols. Some packages in x/crypto don't clear that bar anymore, and I propose we freeze and deprecate (but not remove) them.
The intended outcomes are
All open and future issues and CLs that are not security relevant will be closed linking to this issue.
/cc @agl with whom we discussed this at RWC, and @golang/proposal-review for approval
Why
There are different reasons for deprecating different packages, and I give a short explanation for each of them below, but here's a summary.
What packages
openpgp/otr/— a messaging security protocol from 2000's, mostly obsoleted by the Signal double ratchet and other messaging protocols like OMEMO and MLS, and a very rarely used packagexts/— a block mode that should only ever be used for full disk encryption, which unsurprisingly no one is implementing in Go, and I honestly didn't even know it was in hereI was also hoping to kill pkcs12/ because it's a legacy file format that relies on primitives even worse than the ones listed above, and the package only does decoding while a better fork on GitHub also does encoding, but it has way too many users so that's only been frozen instead.
The text was updated successfully, but these errors were encountered: