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

RFE: execute rpmbuild tests as a regular user #3005

Open
pmatilai opened this issue Mar 28, 2024 · 6 comments
Open

RFE: execute rpmbuild tests as a regular user #3005

pmatilai opened this issue Mar 28, 2024 · 6 comments
Labels
RFE test Testsuite-related

Comments

@pmatilai
Copy link
Member

While investigating #2519 I realized that we're running the entire test-suite as root. That's not how rpmbuild is intended to be used, and masks various permission issues real-world users have, such as that #2519 which is not reproducable in the test-suite because of this.

Adding a user to the test-environment is trivial enough, actually running the tests with it probably less so. Besides all rpmbuild tests, there's probably a lot of other tests too that would optimally run as non-root.

@pmatilai pmatilai added RFE test Testsuite-related labels Mar 28, 2024
@dmnks
Copy link
Contributor

dmnks commented Mar 28, 2024

Yup, good point. I think we should actually make the rpmtests script (which runs in the podman container) run as a regular user there. The individual tests aren't supposed to write to the root filesystem anyway (which we prevent by making it read-only) so being root shouldn't be necessary either. Typically, the tests would run rpm to query something and/or prepare the $PWD (including e.g. using rpmbuild to build the test packages), all of which can be done under a regular user.

Then, when a test actually needs to write to the system, e.g. to install a package (and thus needs root privileges), we can escalate it (via sudo perhaps) in the runroot* functions. This is also how Toolbox works - the user in the container is mapped to your native user on the host and you have to use sudo in the same way as you'd on the host.

So, that means, those tests that run rpmbuild will no longer run as root (unless prefixed with runroot of course).

@dmnks dmnks self-assigned this Mar 28, 2024
@dmnks
Copy link
Contributor

dmnks commented Mar 28, 2024

Bonus point - runroot will finally live up to its name 😄

@pmatilai
Copy link
Member Author

Sounds like a plan 👍

pmatilai added a commit to pmatilai/rpm that referenced this issue Mar 28, 2024
Packages may contain files without write permissions, read-only sources
and whatnot. Run %_fixperms on the whole builddir before trying to
remove it to ensure we don't fail doing our household duties.

It's worth noting that prior to b5f6c64
the build could similarly fail while running it's default %clean.
If we hadn't removeed the default %clean, we'd have to do this
permission fixup first redundantly for the build root and then for the
rest of the %builddir.

Also worth noting is that the added test doesn't actually reproduce the
issue in the test-suite because it runs as root and root is not bothered
by such petty issues as missing read/write permissions to remove. But
at least we have a reproducer for the case once rpm-software-management#3005 is done.

Fixes: rpm-software-management#2519
pmatilai added a commit to pmatilai/rpm that referenced this issue Mar 28, 2024
Packages may contain files without write permissions, read-only sources
and whatnot. Run %_fixperms on the whole builddir before trying to
remove it to ensure we don't fail doing our household duties, both
on post-cleanup but also before trying to remove an old run, as
eg 'rpmbuild -bi' will not run cleanup.

It's worth noting that prior to b5f6c64
the build could similarly fail while running it's default %clean.
If we hadn't removeed the default %clean, we'd have to do this
permission fixup first redundantly for the build root and then for the
rest of the %builddir.

Also worth noting is that the added test doesn't actually reproduce the
issue in the test-suite because it runs as root and root is not bothered
by such petty issues as missing read/write permissions to remove. But
at least we have a reproducer for the case once rpm-software-management#3005 is done.

Fixes: rpm-software-management#2519
@xsuchy
Copy link
Member

xsuchy commented Apr 3, 2024

we're running the entire test-suite as root.

I believe this is not true. I see no code in rpmbuild that would elevate UID to root. Nor any consolehelper. Nor setuid bits.

@dmnks
Copy link
Contributor

dmnks commented Apr 3, 2024

What we mean here is rpm's own test-suite:

$PODMAN run --privileged -it --rm --read-only --tmpfs /tmp -v $vol \

@pmatilai
Copy link
Member Author

pmatilai commented Apr 4, 2024

I believe this is not true. I see no code in rpmbuild that would elevate UID to root. Nor any consolehelper. Nor setuid bits.

Root, in the container.

pmatilai added a commit to pmatilai/rpm that referenced this issue Apr 4, 2024
Packages may contain files without write permissions, read-only sources
and whatnot. Run %_fixperms on the whole builddir before trying to
remove it to ensure we don't fail doing our household duties, both
on post-cleanup but also before trying to remove an old run, as
eg 'rpmbuild -bi' will not run cleanup.

It's worth noting that prior to b5f6c64
the build could similarly fail while running it's default %clean.
If we hadn't removeed the default %clean, we'd have to do this
permission fixup first redundantly for the build root and then for the
rest of the %builddir.

Also worth noting is that the added test doesn't actually reproduce the
issue in the test-suite because it runs as root and root is not bothered
by such petty issues as missing read/write permissions to remove. But
at least we have a reproducer for the case once rpm-software-management#3005 is done.

Fixes: rpm-software-management#2519
pmatilai added a commit that referenced this issue Apr 4, 2024
Packages may contain files without write permissions, read-only sources
and whatnot. Run %_fixperms on the whole builddir before trying to
remove it to ensure we don't fail doing our household duties, both
on post-cleanup but also before trying to remove an old run, as
eg 'rpmbuild -bi' will not run cleanup.

It's worth noting that prior to b5f6c64
the build could similarly fail while running it's default %clean.
If we hadn't removeed the default %clean, we'd have to do this
permission fixup first redundantly for the build root and then for the
rest of the %builddir.

Also worth noting is that the added test doesn't actually reproduce the
issue in the test-suite because it runs as root and root is not bothered
by such petty issues as missing read/write permissions to remove. But
at least we have a reproducer for the case once #3005 is done.

Fixes: #2519
@dmnks dmnks removed their assignment Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFE test Testsuite-related
Projects
Status: Todo
Development

No branches or pull requests

3 participants