Although 1.0.1. is the oldest version currently covered by Bitcoin Core’s extensive CI, Debian Bookwork ships 0.9.2 and #194 introduces test coverage for even older versions. 0.7 has been required since #88.
The CI run of https://github.com/Sjors/bitcoin/pull/100 @ 3d55222 previously checked Bitcoin Core CI against 1.0.1 as the minimum. Lowering the minimum further should not be a problem for that CI.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | ryanofsky |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Even if we want to go lower, older than v0.8.1 wouldn’t make sense if it has a CVE: https://github.com/bitcoin/bitcoin/issues/33176#issuecomment-3180162395
Ubuntu 22.04 LTS is still getting security updates, so they could still bump. I asked here how to make that happen: https://answers.launchpad.net/ubuntu/+source/capnproto/+question/822317
For <0.8.1 you’re referring to this one? https://www.cve.org/CVERecord?id=CVE-2022-46149
1.0.1 also has a CVE: https://www.cve.org/CVERecord?id=CVE-2023-48230
Though you’re not using any web socket functionality?
As @fanquake points out, Debian Bookworm is at 0.9.2.
https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3180548357
So I could go lower, but then we have to either rely on manual testing or add Debian to the CI.
In that case maybe 0.8.2 is a better choice, given hopefully Ubuntu 22.04 will bump to that.
Lowered the documented minimum capnproto to 0.9.2 to support Debian Bookworm (manually tested as part of Bitcoin Core).
Unless there’s another distro out there that has 0.8.2, I think we should hold off on lowering further until Ubuntu 22.04 LTS actually ships it.
Code review ACK b8d438736928c92393bec8ac3123477714d0329b
re: some of the comments above, I don’t think the CVE should be a factor in what minimum version is required because the fixes listed in https://github.com/capnproto/capnproto/security/advisories/GHSA-qqff-4vw4-f6hx go all the way back to 0.5.
IMO, it’d be nice with #194 to only require version 0.7 or later, so IPC just works without headaches on a wide variety of OS’s including Ubuntu 22.04. Version 0.7 has been required since #88.
But this PR looks goods good as-is and unless there are any objections I’ll merge it soon. Versions of 0.7 and 0.8 already don’t work currently due to the bug in https://github.com/bitcoin/bitcoin/issues/33176, so this PR is just accurately documenting current requirements.
Ok, if you think the CVE can safely be worked around, then I’m fine with supporting older versions. And good point about it not being easy to rule out specific versions.
I would sleep a bit better if this repo had test coverage for these older versions, which #194 also introduces.
Note that 1.0.1 is the oldest version currently covered by Bitcoin
Core's extensive CI.
Ok, if you think the CVE can safely be worked around, then I’m fine with supporting older versions.
I think I need to read more to know if we are affected. https://capnproto.org/news/2022-11-30-CVE-2022-46149-security-advisory.html says “The vulnerability is exploitable only if an application performs a certain unusual set of actions.” and I haven’t looked into what those are.
Regardless, I think the build should refuse to use any version affected by the CVE. It will just require a custom check that I can add in #194. I think I’ll go ahead and merge this now to work on that. Thanks for the PR!