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 SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

Am 05.04.24 um 13:39 schrieb Olaf Hering:

Thu, 4 Apr 2024 11:53:45 +0200 Michael Pujos:

...

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

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

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

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

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

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...

[2] https://github.com/jupyter-server/pycrdt

- Ben

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

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.

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

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

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

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

On Sat, Apr 6, 2024 at 3:02 AM Tony Walker 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)