> > 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
> > 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
> 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
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
> 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
@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
@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
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
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
@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
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
@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
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:
@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 */
+
@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) {
-
@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,
@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
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
@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:
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
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
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
@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 */
+
@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) {
-
@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. */
@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,
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:
> 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
>
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
@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:
@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:
> @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
>
@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
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
34 matches
Mail list logo
