-
-
Notifications
You must be signed in to change notification settings - Fork 17.7k
libxml2: split to multiple versions, init libxml2_13, add patch for 5 CVEs #421740
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
Conversation
alyssais
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for picking this up! Looks good to me at a glance, but I'll give the package maintainer a chance to comment. Want to also change all the packages currently overriding libxml2 to use this?
|
Have any of the packages using the old version been tested with the new one, or is it just build errors from the changed soname? I know the ABI changed along with security fixes, but it seems it would be better to avoid the security issues and version proliferation if it ends up working in practice. |
Thanks, but I want to address those packages in separate PRs. My reason is I'm planning to do other refactoring changes for those packages, and including those changes here would be too much for reviewers and probably delay getting this core change merged |
@emilazy The example for the vendored ( I just tested the example for the nixpkgs ❯ ./result/bin/packettracer8
/nix/store/qf1jkmmnwxc9y8wafxk41vr06gkplpsd-ciscoPacketTracer8-unwrapped/opt/pt/bin/PacketTracer: /nix/store/qf1jkmmnwxc9y8wafxk41vr06gkplpsd-ciscoPacketTracer8-unwrapped/lib/libxml2.so.2: no version information available (required by /nix/store/qf1jkmmnwxc9y8wafxk41vr06gkplpsd-ciscoPacketTracer8-unwrapped/opt/pt/bin/libQt5WebEngineCore.so.5)
/nix/store/qf1jkmmnwxc9y8wafxk41vr06gkplpsd-ciscoPacketTracer8-unwrapped/opt/pt/bin/PacketTracer: /nix/store/qf1jkmmnwxc9y8wafxk41vr06gkplpsd-ciscoPacketTracer8-unwrapped/lib/libxml2.so.2: no version information available (required by /nix/store/qf1jkmmnwxc9y8wafxk41vr06gkplpsd-ciscoPacketTracer8-unwrapped/opt/pt/bin/libQt5WebEngineCore.so.5)
/nix/store/qf1jkmmnwxc9y8wafxk41vr06gkplpsd-ciscoPacketTracer8-unwrapped/opt/pt/bin/PacketTracer: /nix/store/qf1jkmmnwxc9y8wafxk41vr06gkplpsd-ciscoPacketTracer8-unwrapped/lib/libxml2.so.2: no version information available (required by /nix/store/qf1jkmmnwxc9y8wafxk41vr06gkplpsd-ciscoPacketTracer8-unwrapped/opt/pt/bin/libQt5WebEngineCore.so.5)
/nix/store/qf1jkmmnwxc9y8wafxk41vr06gkplpsd-ciscoPacketTracer8-unwrapped/opt/pt/bin/PacketTracer: /nix/store/qf1jkmmnwxc9y8wafxk41vr06gkplpsd-ciscoPacketTracer8-unwrapped/lib/libxml2.so.2: no version information available (required by /nix/store/qf1jkmmnwxc9y8wafxk41vr06gkplpsd-ciscoPacketTracer8-unwrapped/opt/pt/bin/libQt5WebEngineCore.so.5)
/nix/store/qf1jkmmnwxc9y8wafxk41vr06gkplpsd-ciscoPacketTracer8-unwrapped/opt/pt/bin/PacketTracer: /nix/store/qf1jkmmnwxc9y8wafxk41vr06gkplpsd-ciscoPacketTracer8-unwrapped/lib/libxml2.so.2: no version information available (required by /nix/store/qf1jkmmnwxc9y8wafxk41vr06gkplpsd-ciscoPacketTracer8-unwrapped/opt/pt/bin/libQt5WebEngineCore.so.5)
/nix/store/qf1jkmmnwxc9y8wafxk41vr06gkplpsd-ciscoPacketTracer8-unwrapped/opt/pt/bin/PacketTracer: symbol lookup error: /nix/store/qf1jkmmnwxc9y8wafxk41vr06gkplpsd-ciscoPacketTracer8-unwrapped/opt/pt/bin/libQt5WebEngineCore.so.5: undefined symbol: valuePush, version LIBXML2_2.4.30 |
|
Gotcha. Then this seems fine. I hope we can do undo it and don’t end up with five |
c38ba8c to
2da008e
Compare
|
Are there any blockers? I feel like this has got enough reviews from enough people, including the package maintainer Overall it is a refactor with python support removed and a new package, I feel like there's not much risk going forward with this as is Note that there are packages on master with vulnerable, vendored libxml2 (2.13) libraries, which I'd love to start working on. However since the base of those future PRs could change at any time, I don't want to create extra work for myself |
|
I don't think there is any blockers, and i would merge if i could. Though this does need to go through staging, and staging people are aware of this PR. It'll likely be picked up for next cycle, though that cycle will have to wait ~2 weeks going by current schedule (current staging-next is running, 25.05 will be after that). Doesn't mean this can't be merged earlier though. In fact, i believe it should be merged soon, so the backport of the CVE fixes can be picked into the 25.05 cycle. |
fixes NixOS#419634 before NixOS#421740 is merged to master
fixes NixOS#419634 before NixOS#421740 is merged to master
ecdsa is insecure, see tlsfuzzer/python-ecdsa#352 For the libxml2 change, see NixOS/nixpkgs#421740
This is a follow-up for #421204 and #421201, cc @alyssais
This solves the problem of not having a unified way to use the old
libxml2package. For some proprietary packages, it's not feasible to get a newer version oflibxml2working. One package used overrideAttrs, while another package vendored the oldlibxml2. I'll address these in separate PRs after this PR is acceptedThis adds
libxml2_13package with a patch applied to fix the CVEUnrelated to the aforementioned PRs, but I think it's safe to remove python2 support. I couldn't find any usages of it (I grepped for
libxml2\.andlibraries/libxml2)Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.