-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
readme: Update minimal dependencies #9448
base: master
Are you sure you want to change the base?
Conversation
To review this, should we compare these docs to the versions used in |
@sneurlax I'm unsure to understand the question. Do you mean that a thorough review of minimal versions used in |
Sorry @Asurar0, I should have been more clear; it wasn't a question to you specifically but rather to the next reviewer. I would think that these docs should match the actual dependencies used/vendored in This is not a review. |
Shouldn't the minimum version be the minimum version that can be used to compile monero? Maybe we should add a recommended column but for example Libunbound constantly puts of vulnerbility fixes so I assume there is no good "recommneded" version for it other than a recent version. |
Most of these vulnerabilities (libunbound and libzmq) are memory related and are in part mitigated by flags used in Cmake. I would argue however that some are not memory related, and therefore out of scope of the compiler. I don't see any reason to set at minimum version any packages that either have reach EOL or include non-memory vulnerabilities. No one can use the former, and no one would want to use the latter. |
From what I can see this is only a README change, which doesn't change the fact that the code can still be build with older toolchain/with older dependencies. I don't see a huge value in this change because, as @selsta pointed out minimal version is not recommended version (well maybe for glibc it is... but this is out of the scope of this PR), it is more of a information to somebody who is willing to compile monero on a machine running older distribution. I would consider updating contrib/depends more important because it actually changes what version of dependencies most of the people will run on their devices, and for those who build monero manually - I think that it doesn't matter to bump the version in readme if the code is still linkable against older libraries - because those usually contain charry-picked patches from distro maintainers, as can be seen in the CVE-2020-15166 example that you mention - if you use zeromq3 from debian repositories (which is on 4.3.1 currently) you will see a patch for that and other issues: https://sources.debian.org/patches/zeromq3/4.3.1-4+deb10u2/ So I vote for not artificially bumping the version numbers if the code is still compatible with older libraries. |
@MrCyjaneK I concur with your perspective. It may be worthwhile to consider opening a separate PR to revisit and update the minimum version requirements in Concerning GCC 7, I would appreciate if any contributors can confirm its ability to compile monero. |
@Asurar0 We currently use GCC 7 for release builds, so yes. Edit: never mind, C++17 bump hasn't made it into the release branch and building |
In the github build workflow in The workflow includes the installation of the
It seems like there are valid reasons to keep minimal dependencies as is because of linux distribution packaging. Unless GCC is wished to be raised in the future or |
For releases, we use Gitian with an Ubuntu 18.04 based image, see: https://github.com/monero-project/monero/blob/master/contrib/gitian/gitian-linux.yml#L11 Minimum GCC should be bumped to 8 for |
A recommended column seems more harmful than helpful, as such things tend to fall out of step with reality (someone would need to monitor all the libraries and keep the recommendations up to date). Dynamic linking, plus a minimum supported version, lets users choose the best version of a library for their needs, and tends to incorporate security patches at the distro level, where appropriate. I do think a new PR incorporating #9446 into the build process by updating the gitian base image to Ubuntu 20 and using a newer gcc version would address some of the issues raised here: The 18 base image doesn't get security updates, so there's a legitimate concern newer release builds contain too many old libraries with known vulnerabilities. |
@iamamyth We can't bump the base image in Gitian without affecting runtime glibc requirements. #8929 replaces Gitian, uses GCC 12.3.0 and doesn't have this issue.
This is less of a concern for build tools. All libraries we statically link are built with |
contrib/depends has had a lot of work put into it to ensure that monero builds on all systems (or most, heh) contributing there makes the most sense to me and that's why I initially thought that the docs should reflect contrib: because that's our shared workspace for ensuring builds work for all the various supported platforms |
It requires host toolchains. Currently building For reference, the state of
|
Monero should keep minimal versions of dependencies updated to reasonable and secure versions to avoid introducing vulnerabilities in third-party builds and stay coherent with its current building process.
The following packages have been updated in this PR: