#2004 Enabling pm_request in fedoraproject koji
Closed: Rejected 4 years ago by zbyszek. Opened 5 years ago by kevin.

Greetings.

releng has gotten a request in https://pagure.io/releng/issue/7878 to enable the pm_request mock plugin.

This would allow packages to install additional packages in their buildroot as they are building, which would be handy for things like golang packages where the dependencies change frequently.

However, again, for end users this would mean this packages must be built in mock, you cannot use rpmbuild directly anymore.

I'm asking fesco to look at this and decide under what conditions enabling this is acceptable to the project.


pm_request mock plugin is intended for use locally on packager machines. It was never designed for use in Koji.

Enabling mock pm_request in Koji is equivalent to giving everyone with FAS account root access on Koji builders. pm_request could be used to execute arbitrary DNF commands, for example to install arbitrary packages on builder and execute their scriplets with root privileges. (Koji builders may not have internet access, but attacker could still embed packages in SRPM submitted as scratch build and install them from local filesystem.)

also this means to enable internet access for rpmbuild, though this can be mitigated by pm_request plugin but it's not there. also this would essentially mean that no packages can be rebuilt locally without internet and it would break koschei ;)

Hi Kevin

Of course the result also builds in rpmbuild. pm request is only used to get BuildRequires in the build root, in rpmbuild mode the packager installs the BuildRequires manually.

It's not as integrated as one would like ideally, but as I stated to releng, the aim is also to start improving things rpm-side, since nothing will happen in rpm if Fedora does not make the first move.

@mizdebsk

I appreciate being paranoid, but really, if your threat model is ability by packager to request the installation of Fedora packages on the builder he can already do that with BuildRequires, pm request does not change a thing.

And if your threat model is ability by packager to execute scriptlets during rpmbuild, he can also already do that, just put any macro in Release or Sources for example (as is commonly done in Fedora to compute shortcommits) and the builder will execute it during rpmbuild too.

So concretely, from an attack surface point of view, it does not change a thing, you need packagers to be responsible, you need Fedora packages to be free of malicious code, you need to log what happens in builds (and mock-install will get every step of the pm request logged like the rest. And only allows dnf installs BTW. As I wrote in the releng issue, pm request is a tad more flexible than what is needed for Go, but I assume you need the flexibility Java SIG side).

@ ignatenkobrain

also this means to enable internet access for rpmbuild, though this can be mitigated by pm_request plugin but it's not there.

Not really, that depends entirely how dnf is setup on the builders.

Even if there's absolutely no security rule on the builders (which I do not believe) you will only get internet access if the definition of Fedora repositories on builders uses Internet access. And then, only to the hosts you put in this definition. And if Fedora repositories are materialized on builders via a nfs or gluster mount (which was the case at some point, no idea how it is now) you only get dnf file: access to this mount points.

So, concretely, it all depends on the dnf config releng chooses to deploy on builders. pm request does not change this config.

Without pm_request authenticated Koji user (anyone with FAS account, not necessarily packager) can execute arbitrary code as unprivileged user inside mock chroot. Packagers (and a few other groups with SCM write access) can execute arbitrary code as root in mock chroot. Worst case this can lead to some resources being wasted, but attacker can't do much as builders are isolated from the internet (eg. sending spam is not possible) and from other tasks running on the same builder.

Enabling pm_request in Koji would diametrically change the situation. Anyone with FAS account would be able to execute arbitrary code on Koji builders, with root privileges, outsides of any chroot. For example, someone who just created FAS account would be able to install software on builder that would inject malware to every package built on affected builder.

@mizdebsk why are you saying it executes things outside the chroot? It installs Fedora packages using dnf, in the chroot. I'd have noticed a long time ago if it installed things outside the chroot.

But anyway I think there are two different things here:

  • the principle of the thing, which is something for FESCO
  • how we actually do it if FESCO is ok with it (eventually, adding security constrains) which is something for RELENG to work on. And I'm quite sure mock upstream would be happy to help address your concerns if it had a clear Fedora position on the whole thing.

Unless you tell us that installing Fedora packages in the buildroot is intrinsically unsafe, which I don't believe is the case here.

And, also, I don't think there is such a huge security difference between packagers doing things locally on their own machines and doing things in copr/koji. The result of their local processing ends up being fed to copr/koji. Most of the packagers have ssh keys or other Fedora credentials on their build machines. Pushing parts of the package processing to local packager machines does not make it much safer. The big Debian/kernel breaches of some years ago started with packager credentials on their own machines.

So, since pm request and similar spec generation systems are routinely used by Fedora packagers at some point of the Fedora package creation chain, it's high time the function was internalized in Fedora infra, within the scope of Fedora security teams, so it can be made as safe as possible. The current situation where more and more packagers do this gray-computing release processing locally, and no one asks releng to look at it, and since no one looks at it it's not done cleanly and safely, is not good for Fedora.

@nim pm_request lets you run any action to the package manager, and you can do tricks to do arbitrary commands to the host environment by abusing the fact that it's shell.

And, also, I don't think there is such a huge security difference between packagers doing things locally

You've got to be kidding me. There's a world of difference between the two. If you screw up only your computer, the damage is limited to you. If you screw with a Koji builder, you can do a lot of damage to Fedora and all of its users as a whole.

@ngompa: then hardcode install in the variant Fedora deploys. That removes all the other options. That's pretty much what the pm_request client I wrote does.

And, also, I don't think there is such a huge security difference between packagers doing things locally

You've got to be kidding me. There's a world of difference between the two.

That's a bit of a simplistic view. If you can poison a packager system, which is used to regularly feed the builders with packages, you will eventually manage to poison one or several packages, and the builders are installed themselves with those packages. It's not even that hard theoretically, @mizdebsk gave the recipe, all you basically need is to infest a scriptlet or one of the things the scriptlet calls, or add a trigger somewhere on a common Fedora package (and I pass on the damage one could do with the credentials loaded on most packager systems).

So, you see, I can play the security scare game too. Security is a continuum, and security can be fixed, and in fact it has more chances to be fixed right on copr and koji with releng overview than by pretending some part of the packaging toolchain does not exist and even if it does it's ok because it runs on packager systems.

@nim mock runs DNF outsides of chroot. DNF may chroot itself for installing packages, but if the request coming from within chroot tells it not to chroot, DNF will happily not do it and work on the system on which mock runs. I just checked and even the tool you wrote can be used to install packages outsides of chroot, eg mock-install -y --installroot / rnv

@mizdebsk sure but that's just argument filtering and the client is the wrong way to do argument filtering anyway, it needs to be done by infra mock-side. So that's a problem for releng not fesco. Don't tell me it's hard to filter arguments lists in a python codebase. As soon as I have a hour of free time I'll just filter anything that starts with - in mock-install, that should close the hole client-side (though, wrong layer to do it).

So, you see, just by having this conversation we identify things to do better, that everyone was happy to let unaddressed as long as it was "just running on packager systems". That's why I'm asking releng and fesco to acknowledge Fedora already uses things like pm request in its packaging processes, and they need to be managed properly in a secure way by Fedora teams such as releng, or security, and not have this infra-bis growing without review on packager systems because Fedora infra lacks functions necessary to package software in 2018.

And, anyway, this is now requested to FESCO needn't worry or deal with the implementation part:

So, to recap:

Functional FESCO view

Due to deep and widespread changes in the way the software Fedora tries to package is released today, more and more Fedora SIGs have to resort to computing BuildRequires in an automated way. I have no idea how pervasive this gray computing is, but pretty much anything that drops prov or req files in /usr/lib/rpm is likely to use similar methods out of band to compute BuildRequires if the autodeps are used for computer code.

Deploying infra to manage those would increase the productivity of Fedora packagers and lead to more packaging being done (more packages, more complex packages, better cleaner packages, more up-to-date and less obsolete packages).

For very modular ecosystems such as Golang, increasing the productivity is a requirement just to get to the point where existing manpower is sufficient to package properly leaf apps. The alternative is to blindly bundle the Golang universe like upstream does, and completely bypass Fedora packaging guidelines and checks, starting with BuildRequires checks.

Also, as the discussion shows, having releng not involved in this part of the packaging process leads to insecure deployments at least on packager systems, weakening the security of Fedora as a whole.

Releng has received a request to integrate the core components needed to preform BuildRequires computation within copr and koji. Releng is asking FESCO if it should work on the subject.

Threat model

The threat model is the compromission of one or several upstream archives Fedora packagers process, that takes advantage of a hole in the code used to process those archives and compute BuildRequires, to attack one or several elements in the Fedora package creation chain, starting with packager systems, then Fedora infra systems, then Fedora user systems. Attack can take the form of system compromise, or compromised packages, or stolen credentials, or a combination of all of those, starting with any element of the package creation chain and leveraging it to attack the next one.

Threat remediation

  • secure packager systems by moving the processing of untrusted upstream archives within the mock secure environment, using code deployed by official audited Fedora packages
  • harden the mock environment on packager systems
  • don't trust the processing that occurred on packager systems, redo-it on Fedora buildsys, where everything can be inspected and checked against compromises

Minimal technical requirement

Ability to install BuildRequires in the build root before %build, based on processing done in the spec file before %build.

Proposed way forward

  • have releng define and refine an hardened mock/dnf/rpm setup suitable for deployment on Koji/Copr
  • decline this setup on Fedora packager systems

Metadata Update from @jforbes:
- Issue tagged with: meeting

5 years ago

#agreed pm_request discussion is still happening in ticket and should be given another week before we address it.

I'm onboard, +1, with some requirements below.

Essentially, I think @nim is right that this will make package management easier and more automatic, making better use of maintainer time. This is a big opportunity to move the state of packaging forward in Fedora.

Requirements:
1. requests needs to be filtered (on the host side) to only install packages from the distribution (todo in pm_requests) and not allow any kind of network access or other bad dangerous things
2. the access to the socket needs to be secured (todo in mock)
3. the requests need to be logged. I think we want to both log this on the client side (from mock-install) and on the host side (from mock) to reliability.
4. a way to repeat the build without pm_requests active needs to be documented, with packages installed manually e.g. with a list from previous point.

To various issues raised:
- repeatability: pm_requests doesn't fundamentally change anything, it only splits the installation of deps into two or more phases. But as long as the same source repo is used, and as the requests are repeatable, the resulting package set is repeatable. Of course the script in %prep could request a different set of packages, but that's the same as using different ./configure options during build — something to avoid. Similarly, those requests may be resolved to different packages as the available package set evolved. But that is already the case today for normal BR.
- visibility: we can log what is requested and what is being installed in response to those requests.
- security: the pm_requests socket needs to be protected and pm_requests must only do a limited set of operations. I'm surprised that's not already the case.
- the general idea of pregenerating the .spec file using an external tool and putting this in dist-git: it's what we do today, but doing it dynamically in %prep is in better.

@zbyszek If we can enhance pm_request so that the information is recorded in Koji such that replaying the task (as opposed to the build input) can do a reproducible build (with the same build inputs, both dynamic and static), then I'm okay with this, along with your requirements.

This was discussed in today's meeting (2018-11-05):
agreed: current proposal is rejected. FESCo asks for a new proposal which addresses issues 1-3 (+6, 0, 0)
Issues:
1) security issues
2) if the implementation is dependent on RPM changes, those need to be made available to EPEL, or we need a way to disable the feature for EPEL builds
3) local build workflow

Metadata Update from @zbyszek:
- Issue untagged with: meeting

5 years ago

The wording of the rejection is quite unfortunate and the meetings logs do not help much.

Please confirm I understood correctly:

  • local build means:

    • being able to build with rpmbuild without mock.
    • but, this is already the case today¹ in pm_request
    • and, in future rpm enhancement mode, rpmbuild will control more things directly, without relying on an external mock client
  • why would there be a need to explicitly disable a feature in EPEL, if it depends on rpm enhancements not available in EPEL? It will already not work like pretty much everything that depends on an rpm newer than the one EL ships.

  • so that leaves only the security to fix. And fixing means:

    • a pm request mode where the client write a list of requires to the socket
    • mock checks it contains no dnf cli flags, and call dnf to installs the result
    • is that all? Because no software is perfectly secure, so to be workable this issue needs an explicit tofix list

And of course some expression of interest by FESCO to help motivating mock and rpm upstream would have been nice.

¹

  • the spec calls a pm_request client command with a list of dependencies to install in %prep,
  • outside mock+pm request the command is a no-op,
    • so it will work if the deps are already installed on system
    • and if not you see in the rpmbuild -ba output the argument list that was passed to the client command
  • is that not sufficient?
    • otherwise I can make the pm request client command echo back the arguments passed to it by the spec in case of no-op
    • either way it will always be possible to hide the command and its output in a macro, like any other command the a spec calls

This is my understanding more or less:

outside mock+pm request the command is a no-op, so it will work if the deps are already installed on system, and if not you see in the rpmbuild -ba output the argument list that was passed to the client command

For me that is good enough.

why would there be a need to explicitly disable a feature in EPEL [...] ?

I think it is OK if it just does nothing but fails cleanly, e.g. that the build fails with an error about missing packages.

mock checks it contains no dnf cli flags, and call dnf to installs the result

Essentially yes. A whitelist of stuff that is allowed to make sure that no arguments are ever interpreted as switches or local files or URLs or anything like that. (Maybe also use -- to seperate the args?).

Thanks for the clarifications

Essentially yes. A whitelist of stuff that is allowed to make sure that no arguments are ever interpreted as switches or local files or URLs or anything like that. (Maybe also use -- to seperate the args?).

Yes, that was one of my ideas as well, need to check how dnf behaves with --.

Essentially yes. A whitelist of stuff that is allowed to make sure that no arguments are ever interpreted as switches or local files or URLs or anything like that. (Maybe also use -- to seperate the args?).

I would prefer it to be only a list of dependencies and not a CLI command-line and pm_requests to use the DNF API to only install the dependencies without any ambiguity.

Metadata Update from @psabata:
- Issue tagged with: meeting

5 years ago

Status: on my side, from an infra used POW, I've prioritized cleaning up the macros that use this to give some time to infra and tooling upstreams to work on this (rpm + mock).
https://pagure.io/go-rpm-macros/c/53dc537ebf7c8389cb1f8e0bdcc6e6e04430327c

The clean up is pretty much finished and in the final testing stage (copr mass rebuild of existing packages)
https://pagure.io/go-rpm-macros
https://pagure.io/golist
https://copr.fedorainfracloud.org/coprs/nim/go-rpm-macros/builds/

So we'll quickly get to the point where we have a choice between pushing it as-is to FPC, with dynamic BR disabled except for private builds, or working some more on infra to finish enabling them.
https://pagure.io/go-rpm-macros/c/b4e48e0d5a0a150556ce4af0452f3af35df94edf

The drawback of the first option is that it will imply two mass spec & guideline changes (one without dynamic BRs, one with it), instead of doing it all in one go, without wasting packager's time.

Once the downstream macro changes are finished and tested it may free some of my time to try fixing things mock-side, but I don't know the codebase or python well enough to contribute efficiently.

The drawback of the first option is that it will imply two mass spec & guideline changes

Agreed, this definitely far from ideal.

working some more on infra to finish enabling them.
I don't know the codebase or python well enough to contribute efficiently.

Hmm, are you saying that without help, you won't be able to move this forward? I was quitely hoping you'd submit an improved implementation ;)

Hmm, are you saying that without help, you won't be able to move this forward? I was quitely hoping you'd submit an improved implementation ;)

To make this possible, we'd need mock to actually hard-depend on the DNF API and generate a socket to receive install requests from within the chroot. I'm not sure that's possible, though @msuchy could indicate if it is.

The trickier part is supporting bootstrap chroot features, which will be required in the near future for various reasons.

Alternatively, it could be a DNF plugin that mock requires, which would make it easier to include on either the host or the bootstrap chroot.

Hmm, are you saying that without help, you won't be able to move this forward? I was quitely hoping you'd submit an improved implementation ;)

I'm saying that without help, I'll try to push it forward anyway, but I'll probably not be very efficient doing so :(

I'm not sure if the requirement to use API is justified. IIUC, mock just invokes dnf currently, and I think it's reasonable to use the same approach here. Rewriting mock to use API would probably be nice, but should not be something that holds up this (relatively small) feature.

I'm not sure if the requirement to use API is justified. IIUC, mock just invokes dnf currently, and I think it's reasonable to use the same approach here. Rewriting mock to use API would probably be nice, but should not be something that holds up this (relatively small) feature.

It wasn't a problem before because people couldn't generally access the package manager from within the chroot. The commands given were basically generated and mostly fixed, so they were safe. However, that is not going to be the case anymore, so we need to control what is possible very tightly.

To make this possible, we'd need mock to actually hard-depend on the DNF API and generate a socket to receive install requests from within the chroot.

The socket part more or less already works, the part that needs rework and hardening is the mock/dnf interface. I can modify the mock client to send install requests in a single line, or one line per BR, with BuildRequires: prefixing or whatever syntax is preferred mock side

Right now mock does not do any argument parsing it just dumps the line on dnf. Which sort or argues for minimal argument sanitation and filtering unless you want to add a BR argument parser mock-side

FESCo shouldn't be prescribing specific implementation details. In the voted resolution (https://pagure.io/fesco/issue/2004#comment-539224) we set a clear list of requirements. If @nim or somebody else submits the plugin again, it should be reviewed by people with domain specific knowledge.

@nim: on the mock side, all input from the chroot must be treated as untrusted and carefully verified to not confuse dnf. This probably doesn't mean that the plugin needs to "understand" the expressions, it's should be enough to make sure that the arguments will not be misunderstood as options. It'd be good if somebody from the DNF team reviewed it.

To make this possible, we'd need mock to actually hard-depend on the DNF API and generate a socket to receive install requests from within the chroot. I'm not sure that's possible, though @msuchy could indicate if it is.

Everything is possible. Though it is not the direction I would like to see in Mock. I would much rather wait till https://github.com/rpm-software-management/rpm/pull/593 gets into a Fedora.

@msuchy yes, right, that’s why I prioritized other things to give it some time to mature, but it's steadily approaching to the point where I need to choose between pass on dynbr for now/invest in pm request/wait some more for PR 593 to be finished for Fedora Go packages and I have not the faintest idea what's the ETA for PR 593 is.

There are about 700 Go-related specs in Fedora right now, I have ~850 specs in my private repo¹, that's a pretty big multiplication factor for any spec syntax change (and <deity> knows the Fedora specs need a thorough cleanup), it would be awfully nice to have one obviously good option to choose from.

It will probably take about another month for my fellow Go packagers to review and comment on the macro changes, and no one will do radical changes at Christmas time, which means deciding mid January to implement the chosen course before FOSDEM I think.

¹ both overlap, but not 100%, the stuff in my private repo uses dynbrs via pm request, Fedora packages obviously not

Metadata Update from @psabata:
- Issue untagged with: meeting

5 years ago

Anyway just to be clear I wrote a draft master plan for the Go side of Fedora here

Metadata Update from @churchyard:
- Issue tagged with: stalled

5 years ago

At this point I think no one knows when rpm devs will want or be able to work on https://pagure.io/fesco/issue/2096 Their first reaction to https://pagure.io/fesco/issue/2096 was “you filled this without us and it is premature”.

If FESCO has some way to help rpm devs finish https://pagure.io/fesco/issue/2096 quickly, sure, this one can be closed. Otherwise it will have to be done as a stopgap :(

Their first reaction to https://pagure.io/fesco/issue/2096 was “you filled this without us and it is premature”

That's sadly true. But it doesn't mean much, because the reaction to this ticket was quite similar ;|.

For the users of this functionality, i.e. maintainers of packages, both solutions are equivalent. The details differ, because in this one, the spec file "reaches out" to have some extra deps installed, in the other one, the build is aborted and restarted. This is quite different, but the effect of "dynamically" installed deps is reached in both cases. I'm sure both approaches could be made to work, safely and effectively, but I understand why RPM developers prefer the other solution.

At this point in time, it's clear that neither this nor the other implementation is viable for F30. In both cases we're talking about F31 at the earliest. The actual amount of coding work is not that great, the trouble is mostly in design. So the most effective use of everyone's time would imho be to review https://github.com/rpm-software-management/rpm/pull/593 and help to push it through.

So, @nim, it's up to you: we can keep this ticket open as a alternative to #2096, if you have the energy. The momentum seems to be with the other approach, so this most likely would be wasted work.

Should we close this ticket due to inactivity?

Yes. This one doesn't met even basic requirements stated in #2096. And anyway, that ticket is the way to go.

Metadata Update from @zbyszek:
- Issue close_status updated to: Rejected
- Issue status updated to: Closed (was: Open)

4 years ago

Login to comment on this ticket.

Metadata