Replies: 9 comments 10 replies
-
|
Epoch and Version could be defined through macros and any macro can legitimately contain shell or %{lua} expansions. Heck, the whole lines could be declared by the way of macros, and it'd still be completely legal. Sorry but we can't declare existing syntaxes invalid just because a 3rd party tool has problems with it. What we could talk about is some kind of evolutionary version of the spec where extra restrictions are placed, but that wont help with what's out there already. |
Beta Was this translation helpful? Give feedback.
-
That's the thing I'm trying to ask for. Yes, together with (something like) #3471.
True, if we ever want to address the security concerns, we must do it at the distribution level. We need a new packager's policy. |
Beta Was this translation helpful? Give feedback.
-
|
Hi, my two cents.. I think it would make sense to have a way for safe RPM parsing. E.g. by disabling a potentially unsafe features during spec parsing as suggested by this issue. This approach is common in other contexts. For example, XML parsers allow disabling features like DTDs (Document Type Definitions) and external entities to prevent attacks like the Billion Laughs or XXE (XML External Entity) injection. While a safe mode could break parsing for some spec files, this is a policy decision for projects to manage. Many projects and individuals parse RPMs on sensitive, non-ephemeral, or network-connected systems, where the risk of arbitrary code execution from these macros is a serious concern. Having a safe parsing option would be a significant security improvement. |
Beta Was this translation helpful? Give feedback.
-
|
I would like to better understand where this requirement is coming from. Many RPM-based distributions are now self-hosted even at the SRPM build stage. Do you wish to move away from that? Why? Not self-hosting has some quite significant drawbacks: adoption of new RPM features at build time becomes more difficult, for example. Not being able to compute release numbers, source files, and patches dynamically pushes the real development outside of dist-git. Without code generation at spec parsing time, we need to maintain everything explicitly as file lists in the spec file. This results in mandatory conflicts for every dist-git update (at least those that add patch files), serializing development. In other contexts, it's possible to avoid such textual conflicts by dropping appropriately named files into a directory. But natively, rpmbuild does not support picking up subdirectories, or any form of globbing. Hence the need for scripting. |
Beta Was this translation helpful? Give feedback.
-
|
The primary motivation is auditability—our processes should reproducibly convert the input sources into binary artifacts. The ultimate source of truth for sources is dist-git (git + tarball cache). We must ensure that nothing outside of dist-git influences the resulting artifacts (abstracting away the kernel and hardware for now), and it's enough to, e.g., analyze sources that are in dist-git checkout + corresponding lookaside cache. Then, from an auditability perspective, a Source RPM does not help. It is essentially a binary blob—an archive—generated from a given spec file and containing the spec file itself, patches, sources, plus distribution/architecture specific metadata. Importantly, any file or metadata (including the spec file itself) added to the Source RPM can be modified on the fly by Turing-complete macros. This means there is no declarative path from dist-git to a Source RPM if arbitrary code execution cannot be avoided. The issue with rpmautospec (and similar tools) is that they execute Turing-complete RPM macros for every commit up to the most recent Version tag change. Fortunately, rpmautospec only depends on parsing the Epoch/Version, which we can usually read without triggering those Turing-complete side effects. So yes, at the end of the day, we need a clear distribution policies for these two fields. The new RPM mode being requested (which I am not saying must be enforced or made the default) should at least allow the distributions (that care) to enforce such a policy.
But we can still continue doing packaging as we've been doing so far, including rpmautospec -- we just should disallow |
Beta Was this translation helpful? Give feedback.
-
But treating dist-git as the source of truth is a choice. We would we make this choice? Some distributions have outsourced dist-git (or planning to do so). TLS connections to dist-git are terminated by a party that is not even under contract with them. Authentication of Git repository contents relies on online connectivity. That makes dist-git a difficult to choice for a persistent archive mechanism.
But that source RPM is then used in a disconnected environment to build another source RPM, which is then used to build the actual binary packages. So the source RPM still contains evidence of what is going. Contrast this with the dist-git service, which can simply return different Git repository contents at different times (with some effort, even for the same commit hash).
About 5% of Fedora packages use macros in Looking at past commits and executing their contents is certainly surprising behavior. It's entirely possible to have something like rpmautospec without that property, by flipping control around: The not-so-hypothetical rpmautospec replacement needs to determine the package version from the Git commit log (excluding blob contents) and present this information to the spec file—as a macro. Then the dependency on past content goes away (instead there's a dependency on past commit messages).
I really don't want to see this as a default because it just pushes actual distribution development out of dist-git because it's just too cumbersome. Then you just see opaque tarball hashes changing in dist-git, effectively reducing transparency. |
Beta Was this translation helpful? Give feedback.
-
This is similar to how dpkg does it. The version is extracted from the changelog file and it's the only place a complete version exists. Although the changelog file is the source of truth, tools like gdp dch can help generate changelog entries from git commit messages (it seems there's even some support for rpm). |
Beta Was this translation helpful? Give feedback.
-
|
Yep, the dpkg approach is interesting. I've occasionally pondered the possibility of adopting that to rpm somehow, if only to eliminate the silly redundancy between what distro policies commonly require on %changelog entries and the real version. |
Beta Was this translation helpful? Give feedback.
-
|
@fweimer-rh wrote
We did this decision back then, and I don't ask anyone to change it. The cat is out of the bag.
Do you mean that maintainers may start generating a Git hash collisions on purpose?
The contents of source RPM ship modified sources, so we could notice - yes. But how do we detect that this is happening?
Normal text expansion macros in Epoch/Version are just OK! It's OK to continue using
I don't see how. Can you elaborate?
I'm not a Debian expert, but this doesn't seem that bad. It's completely safe to read git commit messages, and a historical variant of some text files. It's not OK to have TC side-effects when doing so. |
Beta Was this translation helpful? Give feedback.
-
|
We can't single out Version and Epoch as somehow being more special than some other elementary tag. I understand that rpmautospec might get by with just those but it just doesn't make any sense on rpm level. It would have to be something like the whole main preamble that needs to be parseable in that restricted mode, and whatever the restrictions actually are... taking away eg %{lua:...} execution would be reaaaally hard to sell after people left and right have been adopting it for the more advanced features. Another thing is that the whole "always use global" of Fedora hits the fan here - %global is evaluated right there at definition time, whether the expansion is actually needed for the preamble or not, and with an alleged noexec mode could fail the parse even if the macro is never used. Such a restricted mode would have to be a distro level policy thing, it's not something you can just do in some corner case and hope the rest is okay. |
Beta Was this translation helpful? Give feedback.
-
I'm requesting an opt-in feature. From the RPM perspective, there's no need to "sell it to users," the build-system folks need to go through this when turning it ON. |
Beta Was this translation helpful? Give feedback.
-
|
Such a mode would have to make sense in the rpm ecosystem big picture anyhow. We're really talking about an evolutionary file format of the spec, rather than some parser feature. |
Beta Was this translation helpful? Give feedback.
-
|
Not sure I understand where the problem is. Can you elaborate on the bigger picture requirements? Consider there's a thing like The build system eventually has to perform a full RPM parsing eventually, including |
Beta Was this translation helpful? Give feedback.
-
|
So you parse it once with getting random garbage back, do decisions based on that and then parse it again, getting something completely different back and this is ... ok? This isn't a step to make the spec saner in my books, it'd be more like yet another layer of duct tape on the already teetering heap of kludges. |
Beta Was this translation helpful? Give feedback.
-
|
Of course that random garbage parse round already happens for buildrequires that might greatly affect the parse outcome, including introduce new buildrequires and whatnot. My big picture is looking at how to eliminate these random garbage parses in a controlled manner, not adding more of them. I guess I'm thinking something like a noexec + error on unknown macros restriction on the preamble section. The actual build stages would only be expanded during an actual build. Currently parts of the build scriptlets are expanded at parse time, parts at build time and it creates some really funky situations. |
Beta Was this translation helpful? Give feedback.
-
I wouldn't use such strong words 😅 executing any code that's under control of the user brings much more randomness, and RPM parser may make the whole environment a garbage. Otherwise, yes, I view this as an acceptable compromise to close the doors for code injection here.
If I understand you well, this is literally what I ask for in #3887 |
Beta Was this translation helpful? Give feedback.
-
|
None of this is possible for distributions to adopt anyway, not the least of which because the Red Hat family has RPMAutoSpec, which (ab)uses the macro engine for In general, it is not reasonable to have a "safe" mode because we've already decided eons ago that the macro engine is the sole mechanism for templating and that RPM needs to handle all kinds of templating requirements. |
Beta Was this translation helpful? Give feedback.
-
|
As described in the other part of the discussion the request is not to limit the default parsing power. |
Beta Was this translation helpful? Give feedback.
-
|
More practically, no (sane) packages would be affected at package build time.
What use-case do you have in mind? Any package example? |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
We need to have a mode for parsing specfiles, namely at least Epoch and Version (for rpmautospec) without calling side effects through
%{lua: }and%().Beta Was this translation helpful? Give feedback.
All reactions