Security practices for packages on build.o.o
Hi, In light of the recent xz supply-side attack [1], I wonder if we should have a discussion about security issues around packages on build.o.o that may be similarly exploited by a packager either acting in bad faith or working from a compromised system themself. Some specific issues that I repeatedly keep thinking of: 1. Packages where spec files do not have traceable source tarballs with full URLs pointing to upstream. I understand that this is already discouraged but not entirely forbidden [2], so a user could have something like ``` Source0: totally_not_an_exploit.tar.gz ``` in their specfile, have the package build and submit it to a devel project and eventually Factory. Unless project or Factory reviewers have the time to untar the sources and carefully study them, which seems an almost impossible burden on either, the tarball could, intentionally or not, carry through an exploit. For sources that do indicate the full URL, I understand that bots in the Factory check the bundled tarballs against upstream ones, and decline them in case of any mismatch. However, for locally generated tarballs with no traceability, there is no check that prevents them from being shipped around as part of the distro proper. For new packages, yes, there is a License check done by SUSE, but not security checks unless the package in question raises specific AUDIT level reviews (like installing polkit privileges etc). This means a packager either acting on bad faith or working from a compromised system could generate a tarball containing an exploit and push it, eventually, to Factory. I hope I am wrong. Perhaps we should reconfigure the Factory bot to forbid non-URL sources from Factory packages entirely. I am not sure how many packages currently have these, but I am fixing one right now. 2. Perhaps it is time to allow 2FA, and indeed make it mandatory, for packagers on build.o.o. 2FA is certainly not hack-proof, but it is better than the simple password based authentication we currently use. This will also cut down on spam comments, that have been slowly growing in number. I noticed that they have been discussing this over on Fedora [3] as well. 3. At the level of prj and Factory reviewers, perhaps we should encourage — and eventually enforce — running `autoreconf -fvi` for all autotools based packages after cleaning up pre-generated build scripts from the tarball in %prep. This could be optimally done with a new rpm macro. I understand that this will not be possible for all packages, and that there are still packages that have weird manually written m4 scripts, etc. However, they are a minority, and we should reconsider whether such poorly maintained packages have a place in our distro any more. 4. And finally, please, please, let there be a 15-day, or even 30-day, waiting period for new users on build.o.o before they can start commenting on projects and packages. This will cut down on stupid spammers and "new version please" pressure creators who simply add noise and affect the workload of packagers and project reviewers, most of whom — myself included — do voluntary work, and are often anyway pressed for time. Our comment reporting system, although good that we now have one anyway, only adds to the workload of project and package maintainers and is rather sub-optimal. It often feels like I am sending my reports to a black hole. Thanks for reading through what turned into a rather long email and best wishes. [1]: https://tukaani.org/xz-backdoor/ [2]: https://en.opensuse.org/openSUSE:Specfile_guidelines#Metadata_Tags [3]: https://pagure.io/fesco/issue/3186 -- Atri
Interesting as I also always wondered about what measures are taken in the OBS infrastructure for building Factory packages to prevent malicious contributions, especially in the area of checking that all the files in project (source, patches, .spec) are legit. For example, it is relatively easy for a new contributor to submit a new package in a devel project then in Factory (and become maintainer), or to submit requests to existing projects. I think it should largely remain that way to not discourage new contributions, but have all the safe guards considered to prevent malicious contributions ? Probably not. I think analyzing all possible scenarios and hardening solutions is inevitable and that all distros are evaluating their security at this moment. On 4/4/24 11:16 AM, Atri Bhattacharya wrote:
Perhaps we should reconfigure the Factory bot to forbid non-URL sources from Factory packages entirely. I am not sure how many packages currently have these, but I am fixing one right now.
How would that work with packages built from a .obscpio archive that was generated from invoking a service manually or locally fetching the source from git ? Such as pango: https://build.opensuse.org/package/show/openSUSE:Factory/pango Are these already checked to verify the .obscpio is legit ?
Michael Pujos wrote:
Interesting as I also always wondered about what measures are taken in the OBS infrastructure for building Factory packages to prevent malicious contributions, especially in the area of checking that all the files in project (source, patches, .spec) are legit.
At least patches and specfiles are text, they are verifiable by reviewers. On the other hand, tarballs that exist solely on build.o.o — and nowhere else — do scare me now.
For example, it is relatively easy for a new contributor to submit a new package in a devel project then in Factory (and become maintainer), or to submit requests to existing projects. I think it should largely remain that way to not discourage new contributions, but have all the safe guards considered to prevent malicious contributions ? Probably not. I think analyzing all possible scenarios and hardening solutions is inevitable and that all distros are evaluating their security at this moment.
Yes, this attach has rather been a shock to the system.
On 4/4/24 11:16 AM, Atri Bhattacharya wrote:
Perhaps we should reconfigure the Factory bot to forbid non-URL sources from Factory packages entirely. I am not sure how many packages currently have these, but I am fixing one right now. How would that work with packages built from a .obscpio archive that was generated from invoking a service manually or locally fetching the source from git ? Such as pango:
https://build.opensuse.org/package/show/openSUSE:Factory/pango
Are these already checked to verify the .obscpio is legit ?
I believe these sources, typically generated by a _service file, are verified against a cpio generated by the Factory bot against the same upstream git commit specified in the _service, but I may be mistaken. At least in theory it is possible to verify these. Best wishes. -- Atri
On Thu, 4 Apr 2024, Michael Pujos wrote:
On 4/4/24 11:16 AM, Atri Bhattacharya wrote:
Perhaps we should reconfigure the Factory bot to forbid non-URL sources from Factory packages entirely. I am not sure how many packages currently have these, but I am fixing one right now.
How would that work with packages built from a .obscpio archive that was generated from invoking a service manually or locally fetching the source from git ? Such as pango:
https://build.opensuse.org/package/show/openSUSE:Factory/pango
Are these already checked to verify the .obscpio is legit ?
There's also the concern of typo-squatting an upstream URL or in the case of github refering to a malicious fork with similar enough name. That would mean keeping a whitelist of domains and projects or at least forcing manual reviews of certain types of URL changes for both Source and Patch references. That a tarball also exists in some form somewhere on the internet isn't so much of a reassurance if you have no means of validating its origin. Richard. -- Richard Biener <rguenther@suse.de> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
Thu, 4 Apr 2024 11:53:45 +0200 Michael Pujos <pujos.michael@gmail.com>:
Are these already checked to verify the .obscpio is legit ?
It should have been rejected right away be the bot. A tag has no meaning, a tag can refer to anything. Only the full 40 char git hash is hard to fake. Olaf
Am 05.04.24 um 13:39 schrieb Olaf Hering:
Thu, 4 Apr 2024 11:53:45 +0200 Michael Pujos<pujos.michael@gmail.com>:
Are these already checked to verify the .obscpio is legit ? It should have been rejected right away be the bot. A tag has no meaning, a tag can refer to anything. Only the full 40 char git hash is hard to fake.
Olaf
Why would the bot reject it? The bot should be able check the result of the tag based checkout with the recorded hash during obs commit. It's the same with Source URL lines in the specfile directly. The Github download URL to a release asset or tag tar archive, which usually corresponds to a version is under control of upstream. The content can change. But then the source validator bot detects a change of the file's hash sum and issues a warning. - Ben
Thu, 04 Apr 2024 09:16:03 -0000 "Atri Bhattacharya" <badshah400@opensuse.org>:
I hope I am wrong.
You are. How would any of what you wrote actually help with the recent CVE? Is the dude who submitted the odd code still on-board? I hope he is. I'm sure all submit requests can not get a full security review. Noone spotted the added dot, for example. Who would be able to do the reviews anyway, at the required scale? You are right with the requirement to run autogen.sh for all projects that use autotools. But, it was said it would not help with that CVE. It may raise the bar for future attempts, if we would actually use our existing tooling and grab fixed commit hashes from somewhere and let OBS internally create the source snapshots. Anyway, thanks for raising the topic. Olaf
Olaf Hering wrote:
Thu, 04 Apr 2024 09:16:03 -0000 "Atri Bhattacharya" :
I hope I am wrong. You are.
How would any of what you wrote actually help with the recent CVE?
Sorry, perhaps you misunderstand me and I was probably not as clear as I should have been, but I am not talking about anything that might have dealt with upstream themselves being compromised. Instead, I am talking about an openSUSE packager acting in bad faith or generating the tarball on their local, already compromised system, for example. Unverifiable tarballs in sources may allow another potential route for supply side attacks specifically targetting a distro, say ours.
I'm sure all submit requests can not get a full security review. Noone spotted the added dot, for example. Who would be able to do the reviews anyway, at the required scale?
True, but again, I am not specifically referring to compromised upstream, but rather a bad faith or compromised openSUSE packager.
You are right with the requirement to run autogen.sh for all projects that use autotools. But, it was said it would not help with that CVE.
Not specifically, but perhaps it might have raised a downstream packager's suspicion.
It may raise the bar for future attempts, if we would actually use our existing tooling and grab fixed commit hashes from somewhere and let OBS internally create the source snapshots.
Yes, great point, I think this is already done by Factory bots for sources packaged using _service files, unless I am mistaken. Best wishes, -- Atri
On Thu, Apr 4, 2024 at 12:16 PM Atri Bhattacharya <badshah400@opensuse.org> wrote:
2. Perhaps it is time to allow 2FA, and indeed make it mandatory, for packagers on build.o.o. 2FA is certainly not hack-proof, but it is better than the simple password based authentication we currently use. This will also cut down on spam comments, that have been slowly growing in number.
How will it interoperate with API (in particular, osc)? Anyway, users are authenticated by the external identity provider so it is rather the question to IDP whether it can support 2FA.
Andrei Borzenkov wrote:
On Thu, Apr 4, 2024 at 12:16 PM Atri Bhattacharya wrote:
2. Perhaps it is time to allow 2FA, and indeed make it mandatory, for packagers on build.o.o. 2FA is certainly not hack-proof, but it is better than the simple password based authentication we currently use. This will also cut down on spam comments, that have been slowly growing in number. How will it interoperate with API (in particular, osc)?
Possibly via tokens like PyPI does.
Anyway, users are authenticated by the external identity provider so it is rather the question to IDP whether it can support 2FA.
Yes, I suppose it is rather a suggestion for SUSE. Best wishes. -- Atri
Le jeudi 04 avril 2024 à 13:46 +0300, Andrei Borzenkov a écrit :
On Thu, Apr 4, 2024 at 12:16 PM Atri Bhattacharya <badshah400@opensuse.org> wrote:
2. Perhaps it is time to allow 2FA, and indeed make it mandatory, for packagers on build.o.o. 2FA is certainly not hack-proof, but it is better than the simple password based authentication we currently use. This will also cut down on spam comments, that have been slowly growing in number.
How will it interoperate with API (in particular, osc)?
Anyway, users are authenticated by the external identity provider so it is rather the question to IDP whether it can support 2FA.
It does. SUSE Internal Build Service (IBS) and IDP are already using 2FA, and osc supports it, either with TOTP or ssh keys. This is mostly a matter of enabling that part on openSUSE side (I'm not sure about the 2FA enrollment part, which might be specific to SUSE internal IT) but I'm no openSUSE Hero. -- Frederic CROZAT Enterprise Linux OS and Containers Architect SUSE
Hi Atri, Thank you for raising this. We have the same problem not only with Source tags but also with Patch. They are sometimes too big to be properly reviewed by the accepting obs maintainer. I always use the GitHub URL (github.com/org/pkg/pull/NNN.patch#/pkg-prNNN-shortdesc.patch) when possible in order to have a stable version. But sometimes you have to apply patches which have not been merged yet. In that case, there might be later additional commits (relevant or not) and the URL will yield a different patch which a bot in factory will complain about. So you leave out the URL and only reference it by comment. Am 04.04.24 um 11:16 schrieb Atri Bhattacharya:
1. Packages where spec files do not have traceable source tarballs with full URLs pointing to upstream. I understand that this is already discouraged but not entirely forbidden [2], so a user could have something like
``` Source0: totally_not_an_exploit.tar.gz ```
in their specfile, have the package build and submit it to a devel project and eventually Factory. Unless project or Factory reviewers have the time to untar the sources and carefully study them, which seems an almost impossible burden on either, the tarball could, intentionally or not, carry through an exploit.
For sources that do indicate the full URL, I understand that bots in the Factory check the bundled tarballs against upstream ones, and decline them in case of any mismatch. However, for locally generated tarballs with no traceability, there is no check that prevents them from being shipped around as part of the distro proper.
I plead guilty as charged. There are many packages of mine where there is a Source without associated URL. This ranges from _source generated rust vendor.tar.xz (Who audits these?) to test data tarballs generated by manual downloads. Spot the connection to the XZ backdoor. Examples: - Source6 of python-skyfield [1] - npm generated modules, (only for testing!): python-pycrdt-websocket [2], python-jupyter-ydoc [3] I imagine it would be hard to audit this automatically or force the review team to look into every single archive. Regards, Ben [1] https://build.opensuse.org/projects/openSUSE:Factory/packages/python-skyfiel... [2] https://build.opensuse.org/projects/openSUSE:Factory/packages/pthon-pycrdt-w... [3] https://build.opensuse.org/projects/openSUSE:Factory/packages/python-jupyter...
Ben Greiner wrote:
Hi Atri,
Thank you for raising this.
We have the same problem not only with Source tags but also with Patch. They are sometimes too big to be properly reviewed by the accepting obs maintainer. I always use the GitHub URL (github.com/org/pkg/pull/NNN.patch#/pkg-prNNN-shortdesc.patch) when possible in order to have a stable version. But sometimes you have to apply patches which have not been merged yet. In that case, there might be later additional commits (relevant or not) and the URL will yield a different patch which a bot in factory will complain about. So you leave out the URL and only reference it by comment.
Agree, this is a very good practice that we should make explicit on the wiki (if we have general consensus): https://en.opensuse.org/openSUSE:Packaging_Patches_guidelines
Am 04.04.24 um 11:16 schrieb Atri Bhattacharya:
1. Packages where spec files do not have traceable source tarballs with full URLs pointing to upstream. I understand that this is already discouraged but not entirely forbidden [2], so a user could have something like ``` Source0: totally_not_an_exploit.tar.gz ``` in their specfile, have the package build and submit it to a devel project and eventually Factory. Unless project or Factory reviewers have the time to untar the sources and carefully study them, which seems an almost impossible burden on either, the tarball could, intentionally or not, carry through an exploit. For sources that do indicate the full URL, I understand that bots in the Factory check the bundled tarballs against upstream ones, and decline them in case of any mismatch. However, for locally generated tarballs with no traceability, there is no check that prevents them from being shipped around as part of the distro proper.
I plead guilty as charged. There are many packages of mine where there is a Source without associated URL.
Ha ha, relax, no one is charging anyone! I think we are all guilty and the faith in trust-based-models that we have had so far has been shaken. Instead, let me take this chance to thank you earnestly for all the help you have accorded me with many packages, python packages in particular.
This ranges from _source generated rust vendor.tar.xz (Who audits these?) to test data tarballs generated by manual downloads. Spot the connection to the XZ backdoor.
As far as cargo vendor.tar.xz tarballs are concerned, I may be mistaken, but it is possible to check these by regenerating them from the `cargo.toml` file in the upstream git repository. I think this is done by the cargo_vendor.service already. The vendor.tar.xz contains sources from various different upstream repositories, but all of them are traceable. So, a downstream packager may find it difficult to maliciously alter this. For data files that concern tests, I think we should simply not run tests that require data files not available from upstream. Sometimes upstream will recommend "run this script to download the data files" and as packagers we do run them on our machines and generate a tarball out of them. I think we should stop doing that. Thanks for your response. Best wishes. -- Atri
Am 04.04.24 um 14:25 schrieb Atri Bhattacharya:
Ben Greiner wrote:
I plead guilty as charged. There are many packages of mine where there is a Source without associated URL.
Ha ha, relax, no one is charging anyone! I think we are all guilty and the faith in trust-based-models that we have had so far has been shaken.
Instead, let me take this chance to thank you earnestly for all the help you have accorded me with many packages, python packages in particular.
Likewise. The point is, as long as somehow trusted and well-known contributors like you and me exhibit such practices, how would the maliciously crafted contribution of an attacker be suspicious in this regard?
This ranges from _source generated rust vendor.tar.xz (Who audits these?) to test data tarballs generated by manual downloads. Spot the connection to the XZ backdoor.
As far as cargo vendor.tar.xz tarballs are concerned, I may be mistaken, but it is possible to check these by regenerating them from the `cargo.toml` file in the upstream git repository. I think this is done by the cargo_vendor.service already. The vendor.tar.xz contains sources from various different upstream repositories, but all of them are traceable. So, a downstream packager may find it difficult to maliciously alter this.
But is this actually executed? Does a bot, not to say a reviewer, really look into vendor.tar.xz and try to reproduce it? I am not too deep into rust packaging, but does cargo_vendor actually create reproducible archives based on the lock files or can vendored packages jump in version? In theory, automake generated source tarballs are also traceable, but in the case of xz nobody noticed that the shipped configure script was not generated by the m4 macros.
For data files that concern tests, I think we should simply not run tests that require data files not available from upstream. Sometimes upstream will recommend "run this script to download the data files" and as packagers we do run them on our machines and generate a tarball out of them. I think we should stop doing that.
Maybe, as an enhancement to the url download and repo checkout services, we need an obs-service where we can specify a script for custom test data downloads. Such a script can then be reviewed at submission time and tested for reproducibility by bots.
Thanks for your response.
Best wishes. -- Atri
Cheers, Ben
On 2024-04-04 13:18, Ben Greiner wrote:
But is this actually executed? Does a bot, not to say a reviewer, really look into vendor.tar.xz and try to reproduce it? I am not too deep into rust packaging, but does cargo_vendor actually create reproducible archives based on the lock files or can vendored packages jump in version?
`cargo vendor` creates the vendor directory that later is compressed by the osc service. If you manually change the content here of later (with a patch), the hash will be different and `cargo build` will complain when comparing with Cargo.lock. To patch a vendored crate you need to annotate it via [patch.crates-io]. This all makes naive attack more evident, but AFAIK I still did not see any idea that will protect us for the kind of attack that XZ suffered, when a rogue maintainer sing compromised upstream tarballs, nor as commented, when a OBS package maintainer decides to add a backdoor in the package. Reviews is the only tool that I can see that can help, but it scales so far.
Am 04.04.24 um 15:38 schrieb aplanas:
On 2024-04-04 13:18, Ben Greiner wrote:
But is this actually executed? Does a bot, not to say a reviewer, really look into vendor.tar.xz and try to reproduce it? I am not too deep into rust packaging, but does cargo_vendor actually create reproducible archives based on the lock files or can vendored packages jump in version?
`cargo vendor` creates the vendor directory that later is compressed by the osc service. If you manually change the content here of later (with a patch), the hash will be different and `cargo build` will complain when comparing with Cargo.lock. To patch a vendored crate you need to annotate it via [patch.crates-io].
But what if you also modify the hash in Cargo.lock? `cargo build` (or python-maturin) during rpmbuild inside OBS has no network connectivity and cannot check against trusted rust repositories. Is there a service/bot on obs that checks for compliance with published crates while connected to network?
This all makes naive attack more evident, but AFAIK I still did not see any idea that will protect us for the kind of attack that XZ suffered, when a rogue maintainer sing compromised upstream tarballs, nor as commented, when a OBS package maintainer decides to add a backdoor in the package.
Atri's, argument from the beginning.
Reviews is the only tool that I can see that can help, but it scales so far.
Again, who reviews vendor.tar.xz? I suspect nobody. - Ben
On 2024-04-04 13:55, Ben Greiner wrote:
But what if you also modify the hash in Cargo.lock?
You will see a patch doing that, if not that means that the tgz has been modified.
This all makes naive attack more evident, but AFAIK I still did not see any idea that will protect us for the kind of attack that XZ suffered, when a rogue maintainer sing compromised upstream tarballs, nor as commented, when a OBS package maintainer decides to add a backdoor in the package.
Atri's, argument from the beginning.
That will be a solution for the OBS case, not for the XZ one.
Reviews is the only tool that I can see that can help, but it scales so far.
Again, who reviews vendor.tar.xz? I suspect nobody.
As commented, cargo itself. To change the lock file you need to change the original tarball, and it will (maybe) make the package source verifier[1] to fail. [1] https://en.opensuse.org/openSUSE:Package_source_verification
Am 04.04.24 um 16:24 schrieb aplanas:
On 2024-04-04 13:55, Ben Greiner wrote:
But what if you also modify the hash in Cargo.lock?
You will see a patch doing that, if not that means that the tgz has been modified.
No, a malicious packager can modify it manually. See below.
This all makes naive attack more evident, but AFAIK I still did not see any idea that will protect us for the kind of attack that XZ suffered, when a rogue maintainer sing compromised upstream tarballs, nor as commented, when a OBS package maintainer decides to add a backdoor in the package.
Atri's, argument from the beginning.
That will be a solution for the OBS case, not for the XZ one.
Atri's argument from the beginning.
Reviews is the only tool that I can see that can help, but it scales so far.
Again, who reviews vendor.tar.xz? I suspect nobody.
As commented, cargo itself. To change the lock file you need to change the original tarball, and it will (maybe) make the package source verifier[1] to fail.
[1] https://en.opensuse.org/openSUSE:Package_source_verification
1. We currently have to work around the service because the fix for the vendor/audit dichotomy is still not released https://github.com/openSUSE/obs-service-cargo_vendor/issues/69 https://github.com/openSUSE/obs-service-source_validator/pull/129 https://build.opensuse.org/request/show/1164389 The fact that it is till possible to check-in to obs with --no-service and submit requests go through staging shows that this is not thoroughly checked server-side. 2. Not even the cargo vendor service claims reproducibility: https://github.com/openSUSE/obs-service-cargo_vendor/issues/73 3. Change your "maybe" to "not": [ben@skylab:…languages:python:jupyter]% osc co python-pycrdt [0] A /home/ben/src/osc/home:bnavigator:branches:devel:languages:python:jupyter/python-pycrdt _service: 100%|###################################################################################################################| Time: 0:00:00 A /home/ben/src/osc/home:bnavigator:branches:devel:languages:python:jupyter/python-pycrdt/_service pycrdt-0.8.17.tar.xz: 100%|#######################################################################################################| Time: 0:00:00 A /home/ben/src/osc/home:bnavigator:branches:devel:languages:python:jupyter/python-pycrdt/pycrdt-0.8.17.tar.xz pycrdt.obsinfo: 100%|#############################################################################################################| Time: 0:00:00 A /home/ben/src/osc/home:bnavigator:branches:devel:languages:python:jupyter/python-pycrdt/pycrdt.obsinfo python-pycrdt.changes: 100%|######################################################################################################| Time: 0:00:00 A /home/ben/src/osc/home:bnavigator:branches:devel:languages:python:jupyter/python-pycrdt/python-pycrdt.changes python-pycrdt.spec: 100%|#########################################################################################################| Time: 0:00:00 A /home/ben/src/osc/home:bnavigator:branches:devel:languages:python:jupyter/python-pycrdt/python-pycrdt.spec vendor.tar.xz: 100%|##############################################################################################################| Time: 0:00:00 A /home/ben/src/osc/home:bnavigator:branches:devel:languages:python:jupyter/python-pycrdt/vendor.tar.xz At revision 5d545a11aef4b08d50094a7e681ed851. [ben@skylab:…languages:python:jupyter]% ls [0] python-pycrdt [ben@skylab:…languages:python:jupyter]% cd python-pycrdt [0] [ben@skylab:…on:jupyter/python-pycrdt]% ls [0] pycrdt-0.8.17.tar.xz pycrdt.obsinfo python-pycrdt.changes python-pycrdt.spec _service vendor.tar.xz [ben@skylab:…on:jupyter/python-pycrdt]% tar xf vendor.tar.xz [0] [ben@skylab:…on:jupyter/python-pycrdt]% rm -r vendor.tar.xz [0] [ben@skylab:…on:jupyter/python-pycrdt]% echo "Extra totally not-suspicious extra file" > vendor/extrafile.txt [0] [ben@skylab:…on:jupyter/python-pycrdt]% head Cargo.lock [0] # This file is automatically @generated by Cargo. # It is not intended for manual editing. version = 3 [[package]] name = "arc-swap" version = "1.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "69f7f8c3906b62b754cd5326047894316021dcfe5a194c8ea52bdd94934a3457" [ben@skylab:…on:jupyter/python-pycrdt]% sed -i s/69f7f8/______/ Cargo.lock [0] [ben@skylab:…on:jupyter/python-pycrdt]% tar cJf vendor.tar.xz Cargo.lock vendor [0] [ben@skylab:…on:jupyter/python-pycrdt]% head Cargo.lock [0] # This file is automatically @generated by Cargo. # It is not intended for manual editing. version = 3 [[package]] name = "arc-swap" version = "1.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "______c3906b62b754cd5326047894316021dcfe5a194c8ea52bdd94934a3457" [ben@skylab:…on:jupyter/python-pycrdt]% rm Cargo.lock [0] [ben@skylab:…on:jupyter/python-pycrdt]% osc status [0] M vendor.tar.xz [ben@skylab:…on:jupyter/python-pycrdt]% osc service runall source_validator [0] python-pycrdt: package depends on rust but does not have cargo_audit configured. See https://en.opensuse.org/Packaging_Rust_Software Rust Source Validator: Failed Aborting: service call failed: /usr/lib/obs/service/source_validator --outdir /home/ben/src/osc/home:bnavigator:branches:devel:languages:python: jupyter/python-pycrdt/tmp_edvo5s_.source_validator.service [ben@skylab:…on:jupyter/python-pycrdt]% codium _service [0] [ben@skylab:…on:jupyter/python-pycrdt]% osc diff _service | cat [0] Index: _service =================================================================== --- _service (revision 5d545a11aef4b08d50094a7e681ed851) +++ _service (working copy) @@ -18,4 +18,7 @@ <param name="compression">xz</param> <param name="update">true</param> </service> + <service name="cargo_audit" mode="disabled"> + <param name="srcdir">pycrdt</param> + </service> </services> [ben@skylab:…on:jupyter/python-pycrdt]% osc service runall source_validator [0] [ben@skylab:…on:jupyter/python-pycrdt]% echo $? [0] 0 Of course cargo build/python-maturin now fails because of the invalid checksum in Cargo.toml, but altering an actual vendored package and updating the checksum properly would avoid that: https://build.opensuse.org/request/show/1164987 The cargo vendoring is only an example. There are other languages and build systems suffering from this, too. - Ben
On 2024-04-04 15:47, Ben Greiner wrote:
[ben@skylab:…on:jupyter/python-pycrdt]% head Cargo.lock
This Cargo.lock is one that is inside the vendor. Inserting this file in the vendor tarball is a decision from the obs cargo_vendor service, not from cargo vendor. This file should be present in the upstream tarball. In this case, the security issue seems to be in the pycrdt project, that does not provide the expected Cargo.lock, so it is not integrated in the pycrdt-0.8.17.tar.xz, that is where it should be. [1] https://github.com/openSUSE/obs-service-cargo_vendor?tab=readme-ov-file#what... [2] https://github.com/jupyter-server/pycrdt
Am 04.04.24 um 19:07 schrieb aplanas:
On 2024-04-04 15:47, Ben Greiner wrote:
[ben@skylab:…on:jupyter/python-pycrdt]% head Cargo.lock
This Cargo.lock is one that is inside the vendor. Inserting this file in the vendor tarball is a decision from the obs cargo_vendor service, not from cargo vendor. This file should be present in the upstream tarball.
In this case, the security issue seems to be in the pycrdt project, that does not provide the expected Cargo.lock, so it is not integrated in the pycrdt-0.8.17.tar.xz, that is where it should be.
The tarball from PyPI contains a Cargo.lock file. Here we are again with legitimate (!) differences from the github repo to release tarballs. But: *It. Does. Not. Matter.* I maliciously changed vendor.tar.xz after creating the initial file with the cargo_vendor service . No source validator or review bot on obs would have detected that I committed something different than what the service created. It probably should, but it is quite expensive in terms of obs server resources.
[1] https://github.com/openSUSE/obs-service-cargo_vendor?tab=readme-ov-file#what...
- Ben
Can I just throw in, that this is something we should maybe take upstream? Just recently I packaged ipp-usb, which needed to go through a security audit [1], where, the audit revealed that the single dependency (gladly it was only one), was different than upstream git _or_ tarball, fetched by obs-service-go_modules. but in that regard it's basically unverifiable without actually running go mod vendor yourself, and verifying that the code is the same. (as only tests were missing, and it was a single dependency, I just packaged it seperately). [1] https://build.opensuse.org/request/show/1164374
aplanas wrote:
This all makes naive attack more evident, but AFAIK I still did not see any idea that will protect us for the kind of attack that XZ suffered, when a rogue maintainer sing compromised upstream tarballs, nor as commented, when a OBS package maintainer decides to add a backdoor in the package.
Leaving aside the rust packaging case for now, I posit that having full URLs for `Source<N>` certainly prevents a rogue build.o.o *packager* from inserting a backdoor into tarballs used for openSUSE package builds when upstream tarballs are clean. I think Factory bots already currently — and have for years — enforce this by explicitly checking the package source tarballs against the upstream indicated either with a full URL in the spec file or via _service. Note again that my intention in this thread is not to directly address the kind of backdoor insertion that was done at the level of upstream repositories for xz, but rather about whether a downstream packager could do something similar even when the upstream repo is clean. Best wishes -- Atri
Ben Greiner wrote:
Am 04.04.24 um 14:25 schrieb Atri Bhattacharya:
Ben Greiner wrote: I plead guilty as charged. There are many packages of mine where there is a Source without associated URL.
The point is, as long as somehow trusted and well-known contributors like you and me exhibit such practices, how would the maliciously crafted contribution of an attacker be suspicious in this regard?
As things stand, I agree that it would not. Enforcing strict traceability of upstream sources (and patches where possible) will, I believe, help to a degree; but, there is no silver bullet especially if an attacker were to do something over on OBS in a manner as planned, meticulous, and eventually back-stabbing as the takeover of xz was. We can only try to make it as difficult as possible to get such a package (or update) into Factory by enforcing checks against upstream (assuming that is clean). Let us also mention that many packagers (generally trusted long-timers) are the reviewers of their own SR's when it comes to devel proj submissions. I know I am for several packages. We should probably rethink our guidelines for making one a maintainer of a devel project package, as in something like 10 contributions to the pkg needed before you may be assigned maintainer-ship. Or have a points system: award points for accepted SR's into Factory, etc. and impose a point threshold before allowing maintainer-ship. Just thinking aloud here. Best wishes. -- Atri
On 2024-04-04 15:54, Atri Bhattacharya wrote:
Ben Greiner wrote:
Am 04.04.24 um 14:25 schrieb Atri Bhattacharya:
Ben Greiner wrote: I plead guilty as charged. There are many packages of mine where there is a Source without associated URL.
The point is, as long as somehow trusted and well-known contributors like you and me exhibit such practices, how would the maliciously crafted contribution of an attacker be suspicious in this regard?
As things stand, I agree that it would not. Enforcing strict traceability of upstream sources (and patches where possible) will, I believe, help to a degree;
But this is the obvious escape, isn't? Patches, in general are not traceable. The main reason is that some are generated after a refresh from quilt, others are heavy adaptations from a fix in the main branch, an undisclosed amount comes from patches borrowed from other distributions, downstream adaptations, etc. There are packages with literally hundreds of patches, with specific tools generated to maintain those patches (some uses a separate git, or ad-hoc scripts to detect conflicts).
We should probably rethink our guidelines for making one a maintainer of a devel project package, as in something like 10 contributions to the pkg needed before you may be assigned maintainer-ship. Or have a points system: award points for accepted SR's into Factory, etc. and impose a point threshold before allowing maintainer-ship. Just thinking aloud here.
A ring of trust was also what was hacked in the XZ project. In two years (what Jia Tan used in total) the normal contributor in OBS would join and quit the project several times. I do not want to sound dismissive. There is something that we must implement in the project to protect us against this kind of scenario, but I still do not see any idea that would work in the XZ project nor in OBS. It is indeed a hard problem.
aplanas wrote:
A ring of trust was also what was hacked in the XZ project. In two years (what Jia Tan used in total) the normal contributor in OBS would join and quit the project several times.
How would that affect anything if we forced checks of package tarballs against upstream? Any backdoor introduced in the package tarball (that is not present upstream) would show up in a diff against upstream's and still get caught. If it is a patch (or multiple patches) we are talking about, I would argue it is more reasonable to expect reviewers to verify these (typically a handful, at most, of) plain text files as opposed to reviewing the who-knows-how-many files that comprise the source tarball. Let us not get into the thought-scape of "nothing works perfectly, so let us do nothing at all." Best wishes -- Atri
On 2024-04-04 13:24, Ben Greiner wrote:
I always use the GitHub URL (github.com/org/pkg/pull/NNN.patch#/pkg-prNNN-shortdesc.patch) when possible in order to have a stable version. But sometimes you have to apply patches which have not been merged yet. In that case, there might be later additional commits (relevant or not) and the URL will yield a different patch which a bot in factory will complain about. So you leave out the URL and only reference it by comment.
You may have missed that unmerged commits from pull requests have an immutable commit ID that you can reference. Andreas
Am 04.04.24 um 14:39 schrieb Andreas Stieger via openSUSE Factory:
On 2024-04-04 13:24, Ben Greiner wrote:
I always use the GitHub URL (github.com/org/pkg/pull/NNN.patch#/pkg-prNNN-shortdesc.patch) when possible in order to have a stable version. But sometimes you have to apply patches which have not been merged yet. In that case, there might be later additional commits (relevant or not) and the URL will yield a different patch which a bot in factory will complain about. So you leave out the URL and only reference it by comment.
You may have missed that unmerged commits from pull requests have an immutable commit ID that you can reference. Andreas
I know that, and I do use it for single commits. But a PR consisting of 10 commits would translate to 10 PatchXX lines and corresponding files. The lazy way always has been to download the whole file for the .patch or .diff URL and push it to obs. My point is that none of this is enforced. Nobody reviews big semi-reproducible patches. - Ben
On Thu, 2024-04-04 at 14:39 +0200, Andreas Stieger via openSUSE Factory wrote:
On 2024-04-04 13:24, Ben Greiner wrote:
I always use the GitHub URL (github.com/org/pkg/pull/NNN.patch#/pkg-prNNN-shortdesc.patch) when possible in order to have a stable version. But sometimes you have to apply patches which have not been merged yet. In that case, there might be later additional commits (relevant or not) and the URL will yield a different patch which a bot in factory will complain about. So you leave out the URL and only reference it by comment.
You may have missed that unmerged commits from pull requests have an immutable commit ID that you can reference.
Referencing full URLS is always nice - also for patches; but we can't enforce it by policy (only recommend) - and of course _service files have the UrL in there (then the Source: in spec is without URL) Typical examples are: * Package maintainers writes a patch to build against an older/newer lib which still carry (upstream might be interested) * upstreams PR is against git/main - we might be on a slightly older version (could be even latest tag) and the patch does not apply cleanly. The packager will base his work off of upstream, but the URL won't be valid * Not everybody uses git - and thus not every patch exists as URL (sourceforge, looking at you!) Adding to the packaging guidelines that full URLS are clearly the recommended way makes sense (githb has proven annoying in this though in the past, as the patch files have not been 100% stable between downloads, even when the PRs were not changed) Cheers, Dominique
Dominique Leuenberger wrote:
On Thu, 2024-04-04 at 14:39 +0200, Andreas Stieger via openSUSE Factory wrote:
On 2024-04-04 13:24, Ben Greiner wrote: I always use the GitHub URL (github.com/org/pkg/pull/NNN.patch#/pkg-prNNN-shortdesc.patch) when possible in order to have a stable version. But sometimes you have to apply patches which have not been merged yet. In that case, there might be later additional commits (relevant or not) and the URL will yield a different patch which a bot in factory will complain about. So you leave out the URL and only reference it by comment. You may have missed that unmerged commits from pull requests have an immutable commit ID that you can reference.
Referencing full URLS is always nice - also for patches; but we can't enforce it by policy (only recommend) - and of course _service files have the UrL in there (then the Source: in spec is without URL)
Typical examples are: * Package maintainers writes a patch to build against an older/newer lib which still carry (upstream might be interested) * upstreams PR is against git/main - we might be on a slightly older version (could be even latest tag) and the patch does not apply cleanly. The packager will base his work off of upstream, but the URL won't be valid * Not everybody uses git - and thus not every patch exists as URL (sourceforge, looking at you!)
These are all examples of patches, in which case I agree there are such corner cases where we might not be able to make them 100% traceable. For SourceN tarballs, on the other hand, I think we could make the guidelines stricter, i.e. require Source<N> to either have a full URL or be generated from _service, while only making it recommended for patches. Could we not? I also wonder what the current state of Factory is. Are there packages in Factory where Source0 itself is completely local, for instance? Best wishes -- Atri
Am 04.04.24 um 16:48 schrieb Atri Bhattacharya:
I also wonder what the current state of Factory is. Are there packages in Factory where Source0 itself is completely local, for instance?
openSUSE:Factory/brlemu But I guess we could just drop that one ;-) -- Stefan Seyfried "For a successful technology, reality must take precedence over public relations, for nature cannot be fooled." -- Richard Feynman
Stefan Seyfried wrote:
Am 04.04.24 um 16:48 schrieb Atri Bhattacharya:
I also wonder what the current state of Factory is. Are there packages in Factory where Source0 itself is completely local, for instance? openSUSE:Factory/brlemu
But I guess we could just drop that one ;-)
There are several, including some up-to-date packages like k3b (for example). I have started sending out sr's for packages where full URLs can be used instead. Best wishes,
On Thu, 2024-04-04 at 15:17 +0200, Dominique Leuenberger wrote:
Referencing full URLS is always nice - also for patches; but we can't enforce it by policy (only recommend) - and of course _service files have the UrL in there (then the Source: in spec is without URL)
Typical examples are: * Package maintainers writes a patch to build against an older/newer lib which still carry (upstream might be interested) * upstreams PR is against git/main - we might be on a slightly older version (could be even latest tag) and the patch does not apply cleanly. The packager will base his work off of upstream, but the URL won't be valid
A possible workaround for this sort of situation is to fork the upstream github project, either in the openSUSE namespace or in a personal namespace, and reference the fork in _service (or in the patch URLs). This greatly simplifies assessing the downstream differences, and simplifies development, too. I suppose we can't enforce this, but we could recommend it. Regards, Martin
I want to shift the discussion to the large number of opportunities for hardening and detection that are fairly easy and that we can do right now. Immutable systems are a great idea and provide ONE layer in good security. However, there are no truly immutable systems. Yes, the root file system in Aeon/Kalpa and Silverblue/Kinoite is mounted read-only, but that can be easily circumvented. Also, I know some people will want to argue with me on this point, I or other people will be happy to prove you wrong. What else can we do? A common story is a program that modifies something in /usr/bin, etc. We can easily prevent that with SELinux. For the vast majority of users, there is NO reason for anything other than RPM/Zypper/transactional-update/rpm-ostree/snapper to modify anything in /usr/bin, etc. You can easily extend that to flatpak for /var/lib/flatpak. If you need to allow another program to modify something in one of the common bin/lib directories, then you can add an exception. Yes, even SELinux can be circumvented and have undisclosed/discovered vulnerabilities. However, using SELinux to confine the number of application that can modify bin/lib files greatly reduces the cross-section, vigilance, and effort required. It would also add one more critical layer to our security model. Another thing that really bothers me is the huge number of applications that have access to the entire filesystem AND the network. As an industry we need to stop writing program like that, but that is a different (lengthy) argument. We can mitigate this for a good number of applications via SELinux and Flatpak. For example, does Kate really need access to the network and my SSH and GPG keys? NO! Again, with some judicial use of tools we already have, we can reduce the cross-section for Aeon/Kalpa/Tumbleweed. These are just two of the things I have been thinking about and trying with my own systems. I am not a sysadmin. I am a programmer. However, I would be happy to help these ideas be implemented. I would like your thoughts and ideas for easy big impact ideas. -- Tony Walker <tony.walker.iu@gmail.com> PGP Key @ https://tonywalker1.github.io/pgp 9F46 D66D FF6C 182D A5AC 11E1 8559 98D1 7543 319C
On Sat, Apr 6, 2024 at 3:02 AM Tony Walker <tony.walker.iu@gmail.com> wrote:
I want to shift the discussion to the large number of opportunities for hardening and detection that are fairly easy and that we can do right now.
Immutable systems are not for security reasons: as long as you have anywhere writeable space, and all "immutable" systems I'm aware of have this, an attacker will always find a way to install his malware. This may stop script kiddies, but not more. I proved that in the past often enough when people told me that XYZ is more secure than MicroOS, since XYZ claims malware will not survive a reboot. And with the current SELinux policies nobody will prevent you from running new scripts/applications, since nobody writes a policy for every single application. With SELinux you can only prevent known existing applications from doing things they shouldn't do. If you want real security, you need remote attestestation. This will verify every executable with a remote, trustable source, if it is on a white- or blacklist and if the executable is modified or not. We provide that already with openSUSE MicroOS today (system roles with keylime during installation), but I doubt anybody is really using that. So the features you need are the, you just need to configure and use it. Thorsten -- Thorsten Kukuk, Distinguished Engineer, Senior Architect, Future Technologies SUSE Software Solutions Germany GmbH, Frankenstraße 146, 90461 Nuernberg, Germany Managing Director: Ivo Totev, Andrew McDonald, Werner Knoblich (HRB 36809, AG Nürnberg)
participants (15)
-
Andreas Stieger -
Andrei Borzenkov -
aplanas -
Atri Bhattacharya -
Ben Greiner -
Dominique Leuenberger -
Frederic Crozat -
Martin Wilck -
Michael Pujos -
Olaf Hering -
Richard Biener -
Richard Rahl -
Stefan Seyfried -
Thorsten Kukuk -
Tony Walker