Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
> > I'm concerned that re-implementing parts of rpm has the potential to double > > the surface area for bugs. I get that writing code in C is more difficult > > and error prone than other languages. > > This has generally been borne out to be true, so I generally will advocate > for people to _not_ take the rpm-ostree/rpm-oxide approach, as it leads to > broken user experiences. The main reason rpm-oxide was written was the number of memory unsafety issues found when I audited the librpm codebase, combined with the slow speed at which they were patched. Qubes OS needed a solution that could protect it from future vulnerabilities. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-860858121___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
> > I'm concerned that re-implementing parts of rpm has the potential to double > > the surface area for bugs. I get that writing code in C is more difficult > > and error prone than other languages. > > This has generally been borne out to be true, so I generally will advocate > for people to _not_ take the rpm-ostree/rpm-oxide approach, as it leads to > broken user experiences. I wrote rpm-oxide because of the large number of memory unsafety bugs found in librpm, together with the slow speed at which they have been patched. I believe rpm-ostree had to reimplement much of librpm because there is no reasonable way to use librpm from a multithreaded program. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-860846911___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
> I'm concerned that re-implementing parts of rpm has the potential to double > the surface area for bugs. I get that writing code in C is more difficult and > error prone than other languages. This has generally been borne out to be true, so I generally will advocate for people to _not_ take the rpm-ostree/rpm-oxide approach, as it leads to broken user experiences. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-860823403___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
At the risk of going completely off topic (for this PR, but it is my PR): One of the recommendations made by @pmatilai is that this code should (ultimately) be compiled against librpm and headers using the public API. I agree: the libs are the canonical implementation. I'm concerned that re-implementing parts of rpm has the potential to double the surface area for bugs. I get that writing code in C is more difficult and error prone than other languages. Bugs are easy to accidentally reproduce. Other changes to librpm won't make it to other environments easily. I'm advocating for spending more time on improving this code base, and providing good wrappers instead of rewriting things. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-857218662___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
> As I've said before in rpm-ostree we already carry a re-implementation of > various chunks of librpm as part of the transactional/anti-hysteresis model. > The concept of carrying "pristine" RPMs has been implemented from the start > by importing RPMs into ostree commits, then using plain old hardlinks. For > `apply-live` updates, that also inherently implements the "unpack all files, > rename() into place" model. Curious: what parts of librpm are *not* reimplemented? In particular, are there any parts (other than rpmdb operations) that are neither reimplemented by rpm-ostree nor by rpm-oxide? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-856967742___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
@malmond77 can you please address the security concerns? Right now, those are a hard blocker. RPM already has unpatched 0day vulnerabilities (mostly out-of-bounds reads and integer overflows) and I am not okay with anything that adds more. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-856967071___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
@cgwalters Thanks for the detailed feedback! You're right: support for both paths is critical. I've tried to minimize changes to the original code. The default path is still the non-cow method. The only way to enable this is to 1) install `python3-plugin-dnf-cow` to instruct `librepo` to transcode during downloads and 2) install the optional plugin package to intercept the file creation/write step. Actually deciding whether to do these depends on both the filesystem support *and* an an appropriate partition layout. Systems that split the root filesystem up will likely not benefit, so I like the idea of making that decision within Anaconda. On a side note: I am still working on this - I have been dealing with non CoW related topics for the past few months. I remain committed to addressing the feedback given to the best of my ability so this is acceptable and mergeable in the future. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-856915198___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
Big picture I'd summarize this as: You're proposing a big alternative way to handle RPMs that only works on reflink-enabled filesystems. But the basic fact is that there's widespread use of non-reflink filesystems (ext4, older xfs) on Linux in general. Even if this code was 100% complete and tested, we'd still have to support the non-reflink case for *years* to come for in-place upgrade support at the least. Don't get me wrong, I'd love to push towards a state were we can try to hard require reflinks in e.g. Fedora derivatives and make system software that just fails without it. And require people using older versions to reprovision with a newer filesystem. But it's hard to underestimate the disruption from that - the benefit has to be really large. As I've said before in rpm-ostree we already carry a re-implementation of various chunks of librpm as part of the transactional/anti-hysteresis model. The concept of carrying "pristine" RPMs has been implemented from the start by importing RPMs into ostree commits, then using plain old hardlinks. For `apply-live` updates, that also inherently implements the "unpack all files, rename() into place" model. This would be a 3rd new thing with a new mix of tradeoffs. Which isn't inherently bad, just noting. Certainly for example, we'd need a way to *force off* reflink support so that the "traditional" path could be tested too. If this ends up as an optional plugin, then that simplifes that aspect because then e.g. Anaconda could only install it if it detected the target filesystem chosen by the user supported reflinks. Anyways beyond that high level, on the details of the code I don't have much to add beyond what the librpm maintainers have. I hope we can figure out how to share more code between these 3 approaches (traditional librpm path, rpm-ostree, RPMCoW). -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-856839607___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
Based on a quick look at rpm2extents.c: - In principle, the digest capability of rpm fd currently in rpmio_internal.h should in fact be public, it just needs a saner API (I don't recall the exact details but there was something in there that is not acceptable for a public API at all) - Working with fundamental rpm package elements (lead, signature, header and payload) would seem like a fairly reasonable thing to be able to do with librpm. (Some of) the existing internal API's would not make sense to expose as-is, but in principle opening that up to a degree would be welcome. They were closed off back in the day because they exposed far too much crazy internals, but for example rpmlead.h is probably quite close to exposable again. ... which AFAICS leaves just the hashing bits, which is something I don't see us exposing in the API (simply because it's a rather special piece and not just a header you'd make available), but that's also by far the easiest part to replace with something else. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-773242323___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
@pmatilai thanks for the feedback. I'm generally in agreement wrt the plugin API and externalizing optional code. I'm left wondering about `rpm2extents.c`: this isn't a plugin, but it does make extensive use of internal functions as they are the canonical implementation. Exposing those in librpm is likely not part of the goals for the regular API. What direction do you recommend I look at here? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-772650404___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
Sorry, I just don't have the bandwidth to deal with this in any sort of detail at this time. What seems clear to me though is that this isn't going to be acceptable in F34, beyond that the crystal ball grows dimmer. That said, what little background processing I've managed to do on this, the more I think this isn't a use-case we'd want to be maintaining going forward in rpm itself. It's forced there right now because the plugin API is not public, and that API probably isn't quite enough to handle this as it is, especially getting the plugin API public is a long-standing todo-item, getting increasingly more important. As are a thousand other things, unfortunately, but at least the plugin API is something we *can* justify spending cycles on because it benefits so much more than just somebody's very special use-case. I don't have time to dig into the details, but as a general guideline, try to approach this from the perspective of enhancing the plugin API to the goal of making this possible without changes to core rpm. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-772410935___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
@pmatilai I'd like to address the remaining work with regards to timelines. I'm really keen on shipping as much as possible for Fedora 34 as it likely benefits the base code used for CentOS 9 Stream. I'm aware there are only a few more days left before Fedora 34 is frozen. I'd like to get my expectations in order, I'd like to know if the primary blockers are primarily related to the plugin API, and addressing the concern on untrusted decompression? If it helps, I'd like to chat on IRC or however you'd like, with a promise to write up a summary of our conversation for the record. Is there anyone else you'd like to weigh in here? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-772092241___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
I've addressed some of the nits. That's the easy bit. For a more general update, see https://bugzilla.redhat.com/show_bug.cgi?id=1915976#c4 . -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-770591559___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
@malmond77 commented on this pull request.
> +#define NOT_FOUND 0
+
+#define BUFFER_SIZE (1024 * 128)
+
+/* magic value at end of file (64 bits) that indicates this is a transcoded
rpm */
+#define MAGIC 3472329499408095051
+
+struct reflink_state_s {
+ /* Stuff that's used across rpms */
+ long fundamental_block_size;
+ char *buffer;
+
+ /* stuff that's used/updated per psm */
+ uint32_t keys, keysize;
+
+ // table for current rpm, keys * (keysize + sizeof(rpm_loff_t))
Also addressed in 91f7284e961cdbecdfec5beedbce03ee2f0fbd85
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1470#discussion_r567521417___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
@malmond77 commented on this pull request.
>
for (i = 0; i < plugins->count; i++) {
rpmPlugin plugin = plugins->plugins[i];
RPMPLUGINS_SET_HOOK_FUNC(fsm_file_pre);
- if (hookFunc && hookFunc(plugin, fi, path, file_mode, op) ==
RPMRC_FAIL) {
- rpmlog(RPMLOG_ERR, "Plugin %s: hook fsm_file_pre failed\n",
plugin->name);
- rc = RPMRC_FAIL;
+ if (hookFunc) {
+ hook_rc = hookFunc(plugin, fi, path, file_mode, op);
+ if (hook_rc == RPMRC_FAIL) {
+ rpmlog(RPMLOG_ERR, "Plugin %s: hook fsm_file_pre failed\n",
plugin->name);
+ rc = RPMRC_FAIL;
+ } else if (hook_rc == RPMRC_PLUGIN_CONTENTS && rc != RPMRC_FAIL) {
+ if (rc == RPMRC_PLUGIN_CONTENTS) {
+ /*
+ Another plugin already said it'd handle contents. It's
undefined how
+ these would combine, so treat this as a failure condition.
+ */
Addressed in 91f7284e961cdbecdfec5beedbce03ee2f0fbd85
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1470#discussion_r567521342___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
@malmond77 commented on this pull request. > @@ -850,10 +852,21 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles > files, char *tid = NULL; const char *suffix; char *fpath = NULL; +Header h = rpmteHeader(te); +const char *payloadfmt = headerGetString(h, RPMTAG_PAYLOADFORMAT); +bool cpio = true; Addressed in https://github.com/rpm-software-management/librepo/pull/222 -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#discussion_r567521110___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
@pmatilai the large file format / PAYLOADDIGESTALT is a good idea for constructing original format rpms that most clients can use. I will spend more time thinking about it. @mlschroe I agree that there should be some way to tell RPM that a(n original) rpm file already passed verify locally. I expect to do this implicitly in rpm2extents by failing the transcode if the verify fails. I will look to add a check early in the verify step that looks for the transcoded signature and if so, short circuits the other digest types, like you said in your second point :) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-758151389___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
I could imagine an option that overwrites the payloadloadformat. Using this does two things: - it tells rpm that the payload has been changed and needs to be unpacked in a different way - it works as a contract telling rpm that the caller has done all the needed data verification and disables payload digests/signatures I prefer to do it with an option so that the header does not need to be modified and therefore header-only signatures still work. I see an additional use case for this: deltarpms. If we have that option we no longer need to construct the compressed payload. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-756734511___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
@lnussel , @malmond77 - if you want to talk about CoW on rpm outside the context of this PR, please just open a ticket here instead of going private email. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-754650373___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
Oh and yet another related remark: nothing against having rpm support reflink where possible, it's actually something I've wanted to do for a long time. Rpm would need to track per-filesystem capabilities somehow (there are several other use-cases for that). Related to that, something mentioned in the fedora devel discussion: unpacking to per-filesystem "temporary" hidden location is something that would be useful to rpm beyond this case. And related to that in turn: a long, long overdue thing is to have rpm first unpack the whole package and only if everything up to that point succeeds, replace existing files in one final swoop. That is not an if but when, so you'll want to make sure you don't build too many assumptions around the current broken file by file unpack + replace operation. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-754647627___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
Another broader thought is that perhaps it might be better to add a new plugin slot for this kind of purpose, which gets the fd as an argument and so doesn't need rpmteFd() which is something I'm not really comfortable in exposing in the external API. That would probably eliminate the need for that special PLUGIN_CONTENT return too. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-754619160___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
I concur with @DemiMarie 's security concerns: we only just got the full payload pre-transaction verification in place *finally* in 4.14.2, but this effectively disables not just that but *all* digest and signature verification for the incoming package (in rpm2extent), which is nothing but an untrusted binary from somewhere AIUI. That's not acceptable, really. Note that you don't need rpmdb to verify signatures, you just need the keys which can be populated from any source you like. It's just the default setup that relies on rpmdb. Another point that might be of relevance is that despite saying so in the payload tag, the payload isn't always "cpio" these days, packages with large files are handled with a different format which only uses an integer as the file "header" in the payload. Which might be more reusable for your purposes, and if that was used for the package originally then the alt payload could perhaps be calculated more easily. I don't remember all the details so might be missing something here, but I think there should be something in that direction... -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-754611698___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
@pmatilai commented on this pull request.
> +#define NOT_FOUND 0
+
+#define BUFFER_SIZE (1024 * 128)
+
+/* magic value at end of file (64 bits) that indicates this is a transcoded
rpm */
+#define MAGIC 3472329499408095051
+
+struct reflink_state_s {
+ /* Stuff that's used across rpms */
+ long fundamental_block_size;
+ char *buffer;
+
+ /* stuff that's used/updated per psm */
+ uint32_t keys, keysize;
+
+ // table for current rpm, keys * (keysize + sizeof(rpm_loff_t))
Please use `/* ... */` comments consistently everywhere. `//` comments have
occasionally slipped into the codebase but the generic style is `/* */`.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1470#pullrequestreview-561736879___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
@pmatilai commented on this pull request.
>
for (i = 0; i < plugins->count; i++) {
rpmPlugin plugin = plugins->plugins[i];
RPMPLUGINS_SET_HOOK_FUNC(fsm_file_pre);
- if (hookFunc && hookFunc(plugin, fi, path, file_mode, op) ==
RPMRC_FAIL) {
- rpmlog(RPMLOG_ERR, "Plugin %s: hook fsm_file_pre failed\n",
plugin->name);
- rc = RPMRC_FAIL;
+ if (hookFunc) {
+ hook_rc = hookFunc(plugin, fi, path, file_mode, op);
+ if (hook_rc == RPMRC_FAIL) {
+ rpmlog(RPMLOG_ERR, "Plugin %s: hook fsm_file_pre failed\n",
plugin->name);
+ rc = RPMRC_FAIL;
+ } else if (hook_rc == RPMRC_PLUGIN_CONTENTS && rc != RPMRC_FAIL) {
+ if (rc == RPMRC_PLUGIN_CONTENTS) {
+ /*
+ Another plugin already said it'd handle contents. It's
undefined how
+ these would combine, so treat this as a failure condition.
+ */
The rpm coding style for multi-line comments is as follow, please use that:
```
/*
*
*
*/
```
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1470#pullrequestreview-561735911___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
@pmatilai commented on this pull request.
> @@ -106,7 +106,8 @@ typedef enum rpmRC_e {
RPMRC_NOTFOUND = 1,/*!< Generic not found code. */
RPMRC_FAIL = 2,/*!< Generic failure code. */
RPMRC_NOTTRUSTED = 3,/*!< Signature is OK, but key is not trusted. */
-RPMRC_NOKEY= 4 /*!< Public key is unavailable. */
+RPMRC_NOKEY= 4,/*!< Public key is unavailable. */
+RPMRC_PLUGIN_CONTENTS = 5 /*!< fsm_file_pre plugin is handling content
*/
NAK for adding what effectively is an internal corner case to the highly
visible RPMRC enum.
RPMRC was originally supposed to be a package open result, but is (mis)used for
all sorts of bad and worse purposes throughout rpm, more likely we should move
the plugins to use a separate error code system.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1470#pullrequestreview-561734509___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
@pmatilai commented on this pull request. > @@ -850,10 +852,21 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles > files, char *tid = NULL; const char *suffix; char *fpath = NULL; +Header h = rpmteHeader(te); +const char *payloadfmt = headerGetString(h, RPMTAG_PAYLOADFORMAT); +bool cpio = true; Rpm uses 0/1 integers for booleans throughout. While C99 is fine as such, to me this lone "bool" only ends up looking out of place. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#pullrequestreview-561732682___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
Haven't had a chance to properly look review and think through the concept etc yet, but a few preliminary review remarks to follow... -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-754592489___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
> Hi! Great to see someone thinking about how to leverage CoW file system > features for RPM :-) I had some ideas on the topic a while ago too, coming > from a different direction. Thought about making the build system produce > (and sign) uncompressed rpms directly > https://lnussel.github.io/2020/07/07/rpm-delta-updates/ That is an awesome writeup. I think we're definitely thinking along the same lines, so I'd love to chat about this. I'll email you directly for a followup. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-754139760___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
Hi! Great to see someone thinking about how to leverage CoW file system features for RPM :-) I had some ideas on the topic a while ago too, coming from a different direction. Thought about making the build system produce (and sign) uncompressed rpms directly https://lnussel.github.io/2020/07/07/rpm-delta-updates/ -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-754025728___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
@malmond77 pushed 1 commit. 58f8d8b0dd50ea850884d8fa734d764aa86890f0 RPM with Copy on Write -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470/files/2d0cda5b237034d46d81597906dcd9f0dbdcab92..58f8d8b0dd50ea850884d8fa734d764aa86890f0 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
@malmond77 pushed 1 commit. 2d0cda5b237034d46d81597906dcd9f0dbdcab92 RPM with Copy on Write -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470/files/72fbf58907d93880c5fc4e376a2ef8d2acf6a4c0..2d0cda5b237034d46d81597906dcd9f0dbdcab92 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
> @DemiMarie : this is an excellent point. There is verification of the whole > rpm file in librepo (see > [rpm-software-management/librepo#222](https://github.com/rpm-software-management/librepo/pull/222)) > and rpm signature verification is done after that, but there remains the > possibility of a rogue mirror offering bad data into a compression library. > > My answer for tonight is that this depends on whether you trust your mirrors. > This proposal assumes you do, and I will update the linked proposal with this > caveat tomorrow. The bottom line: this is opt in, and not intended to be > enabled anywhere by default today. Thank you. I certainly do not trust my mirrors, so this would not work for me. > In the immediate future, I will investigate potential solutions. The one I > came up with tonight involves PAYLOADDIGESTALT or another header with a list > of digests per reasonable sized block of compressed data, pre-decompression. > Example: SHA256 with 1MiB block size would add a trivial overhead, but allow > data to be verified before feeding into a decompression library. This would > require support for a digest of the headers in the repodata's primary.xml[1], > and would in turn be part of the checksum in repomd.xml and the metalink > mirror infra. I am looking at [1] for related reasons. This should be acceptable, but only if the repository metadata was signed and that signature was checked. Currently, Fedora doesn’t sign its metadata, but this may be fixed in the future. Signing the metadata is definitely a good idea for other reasons. > I am definitely open to suggestions. Thank you for this very valuable > feedback. You’re welcome! What if the RPM signature included a signature of the RPM header, and the RPM header included a list of hashes? That would allow `rpmkeys -K` and the remaining RPM signature verification machinery to continue to work as normal, with essentially no loss of security. It would also protect against decompression bomb denial of service attacks. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-752339716___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
@DemiMarie : this is an excellent point. There is verification of the whole rpm file in librepo (see https://github.com/rpm-software-management/librepo/pull/222) and rpm signature verification is done after that, but there remains the possibility of a rogue mirror offering bad data into a compression library. My answer for tonight is that this depends on whether you trust your mirrors. This proposal assumes you do, and I will update the linked proposal with this caveat tomorrow. The bottom line: this is opt in, and not intended to be enabled anywhere by default today. In the immediate future, I will investigate potential solutions. The one I came up with tonight involves PAYLOADDIGESTALT or another header with a list of digests per reasonable sized block of compressed data, pre-decompression. Example: SHA256 with 1MiB block size would add a trivial overhead, but allow data to be verified before feeding into a decompression library. This would require support for a digest of the headers in the repodata's primary.xml[1], and would in turn be part of the checksum in repomd.xml and the metalink mirror infra. I am looking at [1] for related reasons. I am definitely open to suggestions. Thank you for this very valuable feedback. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-752335847___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] RPM with Copy on Write (#1470)
How will package signatures be verified? More specifically, will `rpm2extents` verify the signed digest of files before decompressing them? Otherwise, this seems like a potential security risk, in case there is a bug in the decompression library. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1470#issuecomment-752292624___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
