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

Why were all versions prior to 0.14 of this crate yanked? #774

Closed
sgrif opened this issue Jan 26, 2019 · 30 comments
Closed

Why were all versions prior to 0.14 of this crate yanked? #774

sgrif opened this issue Jan 26, 2019 · 30 comments

Comments

@sgrif
Copy link

sgrif commented Jan 26, 2019

On January 10 all versions from 0.9.4 to 0.14.0-alpha4 were yanked, along with the versions of untrusted that they depended on. This has broken the build of any crate which had a dependency on older versions of ring anywhere in their build. For example, crates.io can no longer accept any pull request which makes any changes to our Cargo.toml, as any attempts to do so will resolve several of our dependencies to ancient versions from before they depended on Ring, since the versions used can no longer be resolved.

Was there a reason for yanking all of these versions? Yanking is typically only done if there is a severe bug or security vulnerability found in a version, but I cannot find any security advisory involving Ring. I also can't find a changelog anywhere to see if there was a critical fix listed in 0.14.

Given the severe effects this has on the ecosystem, could you elaborate on why these were yanked?

@sgrif
Copy link
Author

sgrif commented Jan 26, 2019

Just a note, the reason this affected crates.io is due to our use of an older version of the cookie crate. It looks like even the most recent version of cookie doesn't use 0.14 yet though, so this has effectively broken every Rust web app out there

@briansmith
Copy link
Owner

briansmith commented Jan 26, 2019

There is no way other than yanking them to indicate that they're not supported.

@briansmith
Copy link
Owner

  1. I don't answer questions about old versions of ring unless somebody has a support contract with me.
  2. I happily answer questions about the latest version.

@sgrif
Copy link
Author

sgrif commented Jan 26, 2019

@briansmith Yanking is not intended to indicate that a version is unsupported. It's specifically meant for cases where a version is fundamentally broken (security vuln, syntax error, forgot to include a file, etc).

Having a clear policy for supported versions is more than enough to indicate what is or isn't supported. Using yanking for this purpose does nothing other than break folks builds.

Just to be clear, one of the effects of this is that crates.io (a service which ring directly depends on) is no longer able to update any of our dependencies, and there is currently no way for us to remedy that. Please consider unyanking these versions which do not have a specific version to be yanked.

I don't answer questions about old versions of ring unless somebody has a support contract with me.

I'm certainly not looking for answers to questions about older versions or any sort of support, just that you consider unyanking these versions. I'm also happy to handle it for you if I can get consent to do so.

@briansmith
Copy link
Owner

I only provide the latest version for free. Whether earlier versions are safe to use is up for you to determine. I only recommend people use the latest version. I'm not going to unyank the previous versions unless/until one of my customers asks me to.

@briansmith
Copy link
Owner

briansmith commented Jan 26, 2019

Keep in mind that a lot of the changes in ring are sourced from BoringSSL and they don't provide any guidance on which versions are safe or not safe to use, or which change fixes a security bug or the exploitability of any bug or anything like that. Even OpenSSL upstream from BoringSSL often understates risks of not taking a bug fix or doesn't even notice that one of their bug fixes addresses an exploitable vulnerability. This is one of the main reasons I only support the latest version of ring.

@sgrif
Copy link
Author

sgrif commented Jan 26, 2019

I would love nothing more than to be able to ask you as a customer. Unfortunately crates.io is an open source service we provide free of charge, so we don't have the funds for that.

@briansmith
Copy link
Owner

I would love nothing more than to be able to ask you as a customer. Unfortunately crates.io is an open source service we provide free of charge, so we don't have the funds for that.

Right, and I understand that. My suggestion for people who don't have money is to always use the latest version.

@sgrif
Copy link
Author

sgrif commented Jan 26, 2019

This is one of the main reasons I only support the latest version of ring.

Again, I am absolutely not asking you to support any older versions. Only that you don't break the builds of folks on those versions.

Right, and I understand that. My suggestion for people who don't have money is to always use the latest version.

These are dependencies of our dependencies, many of which are unmaintained. This isn't something that's in our control, and updating all of them ourselves would be a significant undertaking on an already strained team.

I'm clearly not going to change your mind on this, so I'm going to back away from this conversation. I hope you reconsider using yanking like this in the future, as it's incredibly disruptive to existing applications when you do so. Right now someone looking to create a brand new Rust web app will be unable to do so, which severely hampers the adoption of the Rust language.

@briansmith
Copy link
Owner

I am planning to yank 0.13.x soon as well.

Keep in mind that I don't even remember what 0.12.x and earlier versions were like. As soon as I release a new version, I completely stop thinking about the old version. Nobody is keeping track of problems with old versions. If I leave the old versions sitting around on crates.io, there might be a problem with them, and then somebody would blame me for not yanking them due to that problem. So, I just assume that old versions are problematic.

Also keep in mind that I'm trying to get this crate to "1.0" and spending time on older versions distracts from that.

@sgrif
Copy link
Author

sgrif commented Jan 26, 2019 via email

@briansmith
Copy link
Owner

I mean, I will yank the last 0.13.x version, 0.13.5, soon, so only crates with ring = "0.14" or later will work.

bors added a commit to rust-lang/crater that referenced this issue Jan 26, 2019
Switch from ring to openssl

The ring update policy is awful (see briansmith/ring#774), so this switches to a crate that doesn't break existing builds every time a new version is released.
bors added a commit to rust-lang/crater that referenced this issue Jan 26, 2019
Switch from ring to openssl

The ring update policy is awful (see briansmith/ring#774), so this switches to a crate that doesn't break existing builds every time a new version is released.
@briansmith
Copy link
Owner

briansmith commented Jan 27, 2019

OK, so, looking at the feedback I've received on this, I see that the way Cargo handles yanked crates is far from ideal and worse than what I expected based on my earlier experiments. Unfortunately, it's not reasonable for me to leave old versions sitting around on crates.io when I'm not sure if they are safe enough to use, so I don't see how I can avoid yanking older versions when I make any kind of potentially-security-affecting change, including merging upstream BoringSSL changes. Otherwise, I feel like I'm exposing myself to liability issues. In the case of ring 0.12 and older, in particular, I simply cannot remember what security-sensitive improvements have been made since 0.13.0-alpha was released on December 24, 2017, over a year ago.

In the case of 0.13, I know that 0.14 does fix a bug that affects all 0.13 versions, and that's why I'm planning to yank 0.13.5 soon.

AFAICT the solution to the general problem of yanking breaking people's builds is to improve how Cargo handles yanked crates so that people can use yanked crates if they want, and so their builds don't break when they are using a yanked crate, if they don't want their build to break. That's work to be done in the Cargo project, not this project.

@pietroalbini
Copy link

Except some weird corner cases (like crater) I don't think it's wise to allow people to depend on yanked dependencies, since almost every other library uses yanking in a different way.

Usually when a security issue is found in a library the vulnerable versions are yanked and a new semver-compatible point release is released. This means libraries don't need to change anything and binaries just need to cargo update to fix the vulnerability.

With ring it's different though, because when a release is yanked there is not a semver-compatible point release you can upgrade to. Since the fixes are in a semver-incompatible release not only you have to fix possible breaking changes in your code, but you also need to do that for every dependency that uses ring (and some might not be actively maintained anymore).

Also, you unfortunately have to use links = "ring-asm", so the update has to be coordinated with everyone since it's not possible to have multiple ring versions in the same codebase.

Your usage of yanking is different than everyone else, and I don't think it's the best tool to say "use this release at your own risk!".

@dwijnand
Copy link

The documented policy of

We prefer to improve ring's API over keeping ring's API stable. We don't keep old APIs around for the sake of backward compatibility; we prefer to remove old APIs in the same change that adds new APIs.

along with this usage of yank will make this particularly disruptive to the ecosystem. I understand your desire and intent, but I wonder what the best way forward is.

@briansmith
Copy link
Owner

I think the best way forward is (1) quickly finish the current round of improving ring APIs so they are closer to ideal, so that there are fewer breaking API changes in the future, (2) find sponsors who are willing to cover the (non-trivial) costs of maintaining older versions.

@dwijnand
Copy link

Going back over the discussion you previously stated

  • I don't answer questions about old versions of ring unless somebody has a support contract with me.

and later

Unfortunately, it's not reasonable for me to leave old versions sitting around on crates.io when I'm not sure if they are safe enough to use, so I don't see how I can avoid yanking older versions when I make any kind of potentially-security-affecting change, including merging upstream BoringSSL changes. Otherwise, I feel like I'm exposing myself to liability issues.

Rather than disrupt the ecosystem, why not just make it loud and clear which version you support (ie. latest) and continue to not answer questions about old versions of ring?

I'm not a lawyer but it looks like you're covered in terms of liability with that disclaimer at the head of the README. Maybe look for advice or contract a lawyer if you're still concerned? I don't know, but perhaps there are services available for open source software like ring.

@anp
Copy link

anp commented Jan 28, 2019

Hi! Would you consider discontinuing crates.io distribution until you feel confident in publishing a 1.0 API? I understand your policy here and its motivations, but it's squarely at odds with the crates.io model of distribution.

Since a lot of people in the ecosystem apparently didn't fully evaluate the dependency they were taking on, it seems like distribution over git would much more appropriately reflect the relationship one would have with ring as a dependency. It would have made this situation much less severe, since they would have been forced to take on a git dependency themselves.

Is that an idea you'd be open to?

@briansmith
Copy link
Owner

No, the way I'm doing things isn't at odds of the crates.io model of distribution, and most people cope with this strategy with no issue. I regularly update many packages that depend on ring (e.g. Rustls, webpki, webpki-roots, sct.rs, ct-logs, and others that I can't mention here) on every ring release and it's very easy to do. One time I updated something like 38 different third party crates to use the newest version of ring and it took less than an afternoon to write and submit all the PRs. It's easy to deal with.

@briansmith
Copy link
Owner

briansmith commented Jan 28, 2019

From another conversation:

Builds with a lockfile will indeed continue to work as long as the lockfile is present, but Cargo will refuse to update any dependency until the yanked one is removed from the dependency tree, blocking the development.

So, if you are worried about this, make sure your end application has a Cargo.lock checked into version control; then everything should go OK until you need to update a dependency. Then, yes, sometimes (but rarely) you'll need to update ring before you can update another dependency. But, it shouldn't be time-consuming to do so.

@briansmith
Copy link
Owner

Also, it seems like this is in part a regression in Cargo; see rust-lang/cargo#6609 (comment), where they note "I think they're gonna try to tackle this in the near future."

@dwijnand
Copy link

No, the way I'm doing things isn't at odds of the crates.io model of distribution, and most people cope with this strategy with no issue.

Isn't this increase in usage of yank a more recent change? https://github.com/rust-lang/crates.io-index/commits/master?before=326f4ee09eec3d52b4a047a5754d48e6bf7bb7eb+35

I think people are having issues. In part because of a bug in cargo: rust-lang/cargo#6609.

Also:

Then, yes, sometimes (but rarely) you'll need to update ring before you can update another dependency. But, it shouldn't be time-consuming to do so.

I think it'll be quite time-consuming given the way to update ring is to "Read the the commit message, the tests, and the patch itself for each change" of the git log. As the owner of the crate I'm sure it isn't too time-consuming.

@briansmith
Copy link
Owner

In practice what I do when I update other people's crates is:

  1. Bump the ring dependency in Cargo.toml
  2. cargo check and update dependencies on other libraries that depend on ring (e.g. webpki, rustls), in a loop until cargo check gets to the compilation phase.
  3. Until things compile and tests pass, keep doing cargo test and address each compilation failure (or, test failure, though I expect that would be rare) by looking to see how the ring (or Rustls or webpki or whatever) API changed, and adjust the code to cope with the API change. Usually this is an improvement to the code.
  4. Submit a PR to the project.

In general, when I make changes to the semantics of a ring API I try to change the API in some way where compilation would fail, so compiling successfully it a good indicator of success in updating the program to use the updated version of ring. But, of course, I do recommend people keep track of how their dependencies (ring or any other dependency) change between updates.

@briansmith
Copy link
Owner

See also rust-secure-code/wg#16:

Some crates on crates.io pull in vulnerable versions of transitive dependencies that do not have a semver-compatible upgrade path. An example of this is OpenSSL crate which is vulnerable to trivial MitM in versions prior to 0.9. There currently exists a crate with 8000+ downloads in the last 3 months that depends on a vulnerable version of OpenSSL, and this info is not exposed on crates.io in any way, so the crate might keep accumulating unsuspecting users.

That is the main reason that I yank old unsupported versions; I don't want to repeat the problems like that one that people have experienced with rust-openssl and other crates.

In fact, the reason I yanked a bunch of old versions recently was because that was the advice offered to me during a discussion about these issues in the Rust Security Working Group.

Also, me yanking old versions of ring isn't new; I did it in the past too. AFACT, what is new is the Cargo bug that was introduced in 1.26, which elevated depending on a yanked crate from "I get a build warning" into "I get a build failure" when you try to update an unrelated dependency. Please track rust-lang/cargo#6609 for the resolution of that bug. I think once that is fixed, this will go a lot smoother.

@anp
Copy link

anp commented Jan 29, 2019

Sorry to have taken up your time with more commenting -- it should have been obvious that we have an unlikely to be resolved difference of opinions here.

@tomaka
Copy link
Contributor

tomaka commented Jan 29, 2019

I regularly update many packages that depend on ring (e.g. Rustls, webpki, webpki-roots, sct.rs, ct-logs, and others that I can't mention here) on every ring release and it's very easy to do. One time I updated something like 38 different third party crates to use the newest version of ring and it took less than an afternoon to write and submit all the PRs. It's easy to deal with.

The problem is not really updating crates that directly depend on ring.

The problem is in second order dependencies and more. If a crate A depends on B, and B depends on ring, then when I update the version of B in A (which I have to do, otherwise A won't compile anymore after ring is yanked) then I also pull all the breaking changes of B.
This can easily snowball and take more than an afternoon in complex dependency graphs.

@briansmith
Copy link
Owner

This can easily snowball and take more than an afternoon in complex dependency graphs.

I'm happy to solve that problem for Parity and anybody else, through the support contracts I offer. Basically, I give away ring for free and ask in return only that (1) people help each other update to the latest version, or (2) people buy support contracts to support the project financially. Interestingly, of all the organizations who have supported this project financially, all of them are happy to use the latest version.

@briansmith
Copy link
Owner

A PR that attempts to fix rust-lang/cargo#6609 was merged into Cargo and I think it is available in Nightly, though I haven't checked. That issue is still open because, I think, they want to add automated tests for the change. But, you might help them out by checking if the patch fixes the problem. Specifically, with that change you should be able to update a dependency on a crate X even if you're also currently depending on a yanked crate Y, without needing to update the Y crate dependency first.

I believe that will resolve all the issues people have here.

@jhpratt
Copy link

jhpratt commented Mar 1, 2019

@briansmith My understanding from reading this thread is that you have not committed to changing your practice of yanking old versions of the crate; is that correct? I fully support yanking when there is a known vulnerability, but until that point, I see no reason for you to do so. You can clearly indicate in a README or other file that it's not supported without yanking it.

If my understanding is correct (in that you will continue yanking old versions without known vulnerabilities), I have no choice but to use a different library.

Repository owner deleted a comment from icefoxen Mar 1, 2019
@briansmith
Copy link
Owner

As of rust-lang/rust#58522, which I think is included in the newest stable release, Cargo should work better with yanked crates; i.e. the way it used to work when I initially tested out ring's policy here. So make sure you're using the 1.33 version of Cargo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants