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

[request] version bump for common offenders #23277

Open
valgur opened this issue Mar 26, 2024 · 4 comments
Open

[request] version bump for common offenders #23277

valgur opened this issue Mar 26, 2024 · 4 comments
Assignees
Labels
question Further information is requested

Comments

@valgur
Copy link
Contributor

valgur commented Mar 26, 2024

These are some of the most common offenders for version conflicts currently, in my experience. I propose doing a version bump for the listed recipes, about a 100 packages in total. I would add Boost to this list as well, but that would double the number.

I'm willing to create the PRs (at a reasonable rate) if I get greenlit for that. @jcar87, I think you advised against doing it unprompted it in one of the PRs by @mayeut, so I'm pinging you personally.

Alternatively, maybe some of the listed dependencies could be added to the permitted version ranges list.

expat -> [>=2.6.2 <3]

Version ranges for expat are allowed and recommended now: #23510 (comment)

2.6.1 and below are vulnerable
https://nvd.nist.gov/vuln/detail/CVE-2024-28757
https://repology.org/project/expat/information

libxml2 -> [>=2.12.5 <4]

#23276
2.12.4 and below are vulnerable
https://repology.org/project/libxml2/information

imath / openexr

imath/3.1.11: #23275
No vulnerable versions
https://repology.org/project/imath/information

openexr/3.2.3
3.2.1 and below are vulnerable
https://repology.org/project/openexr/information

xz_utils -> 5.6.1 5.4.5

https://repology.org/project/xz/information

sqlite3 -> 3.45.3

3.43.1 and below are vulnerable
https://repology.org/project/sqlite/information

freetype -> 2.13.2

2.11.1 and below are vulnerable
https://repology.org/project/freetype/information

fontconfig -> 2.15.0

2.12.0 and below are vulnerable
https://repology.org/project/fontconfig/information

glib -> 2.78.3

2.75.0 and below are vulnerable
https://repology.org/project/glib/information

@mayeut
Copy link
Contributor

mayeut commented Apr 13, 2024

3 weeks have gone by for this security related issue.
Since then, expat<2.6.2 now all have known security issues.

I went ahead and started to create PRs in order to solve some of those.

I started to bump recipes using expat/2.6.0, if there were conflict with 2.5.0, then they were pre-existing conflicts and will still be there after this first batch.

If the recipe has other dependencies listed here, they'll be bumped as well (libxml2, opener, sqlite3,...) thus it would be ideal to get #23276 merged in order to bump libxml2 to the latest version at the same time.

The order in which to review / merge PRs is as follows:

@jcar87
Copy link
Contributor

jcar87 commented Apr 14, 2024

3 weeks have gone by for this security related issue.
Since then, expat<2.6.2 now all have known security issues.

Please remember that consumers have multiple ways in which to force the Conan solver to solve to a specific version, overriding the values in the recipes:

  • Using overrides at the top level (docs here)
  • Using the new replace_requires functionality (docs here)

Thus, for security-minded users that identify affected versions, this is recommended, along with the use of lockfiles. The most important aspect here is that the version without the security issues is already available. Overriding the versions will cause Conan to identify which recipes need to be built from source against the new versions.

Amending many recipes, while desirable, still does not guarantee that all users will immediately benefit when solving their graphs:

  • They may be using lockfiles that they need to update
  • They may have not done --update yet, and are relying on older revisions from their cache
  • They may be using their own fork and binaries built on their own infrastructure and they haven't sync'ed with upstream yet.
  • They may be explicitly using older versions/recipe revisions of things. We see it very often that "we can't be on the latest version of X because reasons" - so they may not benefit at all.
  • They may be using recipes that can't use the most recent versions of some dependency - which may be causing the solver to bring in an "older" revision/version of something else.

In the same way that we expect users to follow a managed process to bring-in the latest updates, as part of the same process and due diligence they can audit their dependency graph and ensure they are using the versions they require, with overrides/replace_require as mentioned above - thus, it is entirely possible for users to bypass the PR process in Conan Center to ensure their graph has the versions they require.

I went ahead and started to create PRs in order to solve some of those.

I appreciate the sentiment behind this, but please avoid opening this many PRs, as it can overload the CI service. There are already 600 open PRs - opening this many new ones does not really help draw our attention to security-motivated issues.

On the other hand, a few comments:

  • If the PRs require being merged in a specific order, please specify so in very clearly in the description. Otherwise they may get merged out of order, causing conflicts to users, or missing binaries.
  • Please specify in the PR description why other dependencies are being bumped. Some of those will introduce version conflicts that we are trying to avoid - we are no longer accepting version bumps without a reason, or without merging them in the right order to avoid conflicts.
  • "Drop vulnerable versions" from the recipe (config.yml and the likes) really has no effect other than "no new recipe revsions will be published for this version". The versions will still be available and served by the Conan Center remote (we never remove recipe revisions or binary package except in very rare circumstances). Versions should only ever be dropped when no other recipe in the same commit in the repository depends on that specific version - if that is not the case, please mention this in the PR but do not remove the versions. This causes issues not only to us but users that use conan-center-index as a fork (with their own servers).

As for the proposed solutions:

  • Also please note that we will welcome backporting patches to older versions to address security vulnerabilities (where there is trusted, auditable source for that patch) that would mean that specific issues can be addressed in a single PR, rather than this many. This is what the Linux distros have done for many years - it is unlikely that they just bump to an actually new version.
  • In the case of expat and libxml2, we are happy to consider a version range. They seem to fall under the category if API/ABI stable around the major version number - so perhaps there is something to consider there. Will instruct the team to prioritise this approach, as that would make PRs like the ones proposed redundant in the future.

@valgur
Copy link
Contributor Author

valgur commented Apr 18, 2024

Added fontconfig and glib to the list as well.

glib is another good candidate for version ranges use.

@mayeut
Copy link
Contributor

mayeut commented Apr 20, 2024

First of all, thanks for taking care of reviewing / merging the opened PR.

@jcar87,

Thanks for your detail answer. I do understand and appreciate everything you're saying about how users should take care of managing their graph and, that's just true.
However, the unsaid & underlying assumption here, as I understand it, is: "CCI graph is outdated and unsafe by default". This is most likely not true but that's what your answer might suggest. I'm just not at ease with this situation even if only to contribute to recipes that will pull known unsafe dependencies in the process.

"Drop vulnerable versions" from the recipe (config.yml and the likes) really has no effect other than "no new recipe revsions will be published for this version"

Well, it has one major side effect which is to reduce CI / infrastructure load when rebuilding the recipe. Given that CI load is mentioned as an argument not to open too many PRs to bump dependencies, I would say it's not only major but probably critical (doing so respecting the rules you defined in your answer but also the one in

### Removing old versions
The Conan Team may ask you to remove more if they are taking a lot of resources. When removing old versions, please follow these considerations:
* keep one version for every major release
* for the latest major release, at least three versions should be available (latest three minor versions)
Logic associated with removed revisions implies that entries in the `config.yml` and `conandata.yml` files should also be removed. If anyone needs to
recover them in the future, Git contains the full history and changes can be recovered from it.
Please, note that even if those versions are removed from this repository, **the packages will always be accessible in ConanCenter remote**
associated to the recipe revision used to build them.
).

Also please note that we will welcome backporting patches to older versions to address security vulnerabilities (where there is trusted, auditable source for that patch) that would mean that specific issues can be addressed in a single PR, rather than this many. This is what the Linux distros have done for many years - it is unlikely that they just bump to an actually new version.

As I understand it, you choose either stable & secure or unstable & secure when you choose a Linux distro. So while some do backports of security/bug fixes (which can end up, but not necessarily, with "just take the last version and override the version"), others just live at edge, possibly with patches that gets removed once a new version for a package comes out.

Obviously, CCI can't possibly test all combinations, it's probably more likely that a new version will be contributed than a patch for security (especially for patch versions). This leads me to believe that CCI shall probably live at edge for its dependencies.

In the case of expat and libxml2, we are happy to consider a version range.

That's a good thing, thanks !

Another solution that's not been put on the table yet is whether or not it would be possible to create a bump PR with multiple recipes ?

valgur added a commit to valgur/conan-center-index that referenced this issue Apr 20, 2024
valgur added a commit to valgur/conan-center-index that referenced this issue Apr 20, 2024
valgur added a commit to valgur/conan-center-index that referenced this issue May 14, 2024
conan-center-bot pushed a commit that referenced this issue Jun 11, 2024
* proj: bump v6, simplify patching

* proj: fix SQLite version check

* proj: add v9.4.0

* proj: set CMP0077

* proj: tweak patching

* proj: revert old patches, v9.4.0 no longer requires patching

* proj: fix failure due to unrecognized warning flags

* proj: ignore warning flags for GCC 7 as well

* proj: require newer CMake

* proj: bump sqlite3 for #23277

* Added new patch version

* Update recipes/proj/all/conanfile.py

---------

Co-authored-by: PerseoGI <perseog@jfrog.com>
Co-authored-by: Rubén Rincón Blanco <git@rinconblanco.es>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants