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

proxy: Update prost to 0.6 #3963

Closed
olix0r opened this issue Jan 23, 2020 · 5 comments · Fixed by linkerd/linkerd2-proxy#414 or linkerd/linkerd2-proxy#585
Closed

proxy: Update prost to 0.6 #3963

olix0r opened this issue Jan 23, 2020 · 5 comments · Fixed by linkerd/linkerd2-proxy#414 or linkerd/linkerd2-proxy#585

Comments

@olix0r
Copy link
Member

olix0r commented Jan 23, 2020

ID:       RUSTSEC-2020-0002
Crate:    prost
Version:  0.5.0
Date:     2020-01-16
URL:      https://rustsec.org/advisories/RUSTSEC-2020-0002
Title:    Parsing a specially crafted message can result in a stack overflow
Solution:  upgrade to >= 0.6.1
Dependency tree: 
prost 0.5.0

The proxy has dependencies on prost 0.5, as does proxy-api, and tower-grpc-build, etc.

Is it feasible to update this dependency?

olix0r added a commit to linkerd/linkerd2-proxy-api that referenced this issue Jan 23, 2020
@hawkw
Copy link
Member

hawkw commented Jan 23, 2020

I'm working on this. The breaking changes in prost 0.6 shouldn't be a problem for us, but it requires updating to bytes 0.5, which then implies a lot of dependency updates. So, may take a bit.

@hawkw
Copy link
Member

hawkw commented Jan 23, 2020

This will require updating the v0.1.xtokio-io and tokio-buf to pick up the breaking changes to bytes in 0.5, which looks like a significant chunk of work (there are a lot of breaking changes). Since we would like to update the proxy to tokio 0.2 in the near future, a better short term solution may be looking into whether the security fix in prost 0.6.1 can be backported to prost 0.5.x, and then picking up 0.6.1 when the proxy is updated to the tokio 0.2 ecosystem?

@hawkw
Copy link
Member

hawkw commented Jan 23, 2020

Here is a patched prost 0.5.x with the security fix: https://github.com/linkerd/prost/tree/v0.5.x. We can update the proxy to patch all prost deps to use this. Since I believe Dan has stated he doesn't intend to maintain 0.5.x (https://github.com/danburkert/prost/issues/267#issuecomment-575409718) we don't have to worry about missing future patches...especially because we ought to be updated to tokio 0.2 soon and can therefore pick up the latest prost.

hawkw added a commit to linkerd/linkerd2-proxy that referenced this issue Jan 23, 2020
This branch updates the proxy's `prost` dependency to a patched version
of `prost` 0.5 that incorporates the cahnges in danburkert/prost#268.
This patch fixes a security issue where a malicious protobuf message
could be used to trigger a stack overflow.

We are unfortunately unable to easily update to `prost` 0.6.1, which
includes this fix, as 0.6 updates the `bytes` dependency to 0.5. The
`tokio` 0.1 ecosystem that the proxy currently uses still depends on
0.4, and the breaking changes in 0.5 are quite significant. Therefore,
updating to `bytes` 0.5 would require a lot of fairly large changes to
legacy versions of...pretty much everything (`tokio-io`, `tokio-buf`,
`hyper`, `http-body`...). As we intend to update to `tokio` 0.2 in the
near future, patching all these legacy dependencies is a bit of a waste
of time. Therefore, I opted to backport the security fix to a compatible
`prost` version instead.

Closes linkerd/linkerd2#3963

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@olix0r olix0r reopened this Jun 16, 2020
@olix0r
Copy link
Member Author

olix0r commented Jun 16, 2020

Now that we've updated to tokio-0.2, we should ensure that we eliminate our (patched) prost 0.5 dependencies so that cargo audit is happy.

@hawkw
Copy link
Member

hawkw commented Jun 16, 2020

The proxy itself is now using prost 0.6, in fact, now that we've switched to tonic, we can't use 0.5 even if we wanted to. There is still a dependency on 0.5, but it's for the test-support mock controllers, which still use tower-grpc. Hopefully, those should be removeable soon.

olix0r pushed a commit to linkerd/linkerd2-proxy that referenced this issue Jun 30, 2020
This branch updates the test support mock control plane components in
`linkerd2-app-integration` to use `std::future`, Tonic, and Tokio 0.2.
Rather than spawning a separate thread for each control plane componwnr
as we did previously, they are now spawned as tasks on the main test
thread's runtime. As discussed in #580, this _may_ make the tests
slightly less flaky and/or slightly faster on CI.

Closes linkerd/linkerd2#3963

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
2 participants