API improvement to accommodate for RPM CoW (PR#1470)

[chantra]

Related PR: #1470

This is a follow-up of #1470 (comment) .
Below, I have tried to capture the required building blocks to be able to integrate RPM CoW into RPM with limited conditionals in the core RPM codebase. While I suggest some specific plugin hooks, this is not meant to be a prescriptive list and I am open for feedbacks/suggestions to reach the end goal.

RPM CoW minimizes IO by taking advantages of reflinking available in file systems such as BTRFS and XFS.
In order to do this, the data on disk must be layed down as extents which can then be reflink-ed.

To do this, RPM CoW introduces rpm2extents, an RPM utility to take an RPM as input stream and output that same RPM where the payload has been extracted and aligned to form file extents. Some meta data is also added to the end of the file to be able to find at which offset in the transcoded RPM file a given file in the payload is. When installing the file, rpm can then reflink this file instead of copying it. Therefore, the RPM that is on-disk, while having the same content, is not bitwise copy.
Under a typical case, it saves at least 1 IO (1 read when reading the compressed RPM, 1 write when copying files to disk). This becomes more valuable when servers are running container workload, or CI workload (think mock builders) as the same package are installed multiple times from the same cached file. Data is now reflinked (O(1)) instead of being decompressed and copied, saving both IOs and disk space.

This also means that RPM needs to be taught how to install such files. Fortunately, RPM supports plugins which allows augmenting RPM in an optional way and without modifying the core RPM logic. The current API has limitations for the RPM CoW use case though, but we should be able to provide new API entry points to make this work.

RPM CoW changes currently needs a few more hooks to be able to perform its job:

• As part of rpmPackageFilesInstall it needs to provide a custom rpmfi iterator: rpmfilesIter(files, RPMFI_ITER_FWD) instead of uses the content of the archive with rpmfiNewArchiveReader(payload, files, RPMFI_ITER_READ_ARCHIVE) as the list of files/directories and their metadata is now read from the RPM headers metadata vs the cpio archive one. In the current RPM CoW implementation, this is what is being done with plugin_fsm_file_archive_reader_func
• When a file needs to be installed, it needs be given the opportunity to install the file itself (e.g reflink instead of copying from archive) and bypass the original RPM logic. In current RPM CoW implementation it is done by plugin_fsm_file_install_func
• The plugin needs a mechanism to find the file descriptor of the RPM. Currently this is done in exposing a rpmteFd function that given a transaction element returns the fd associated with it.

Another new concept that needs to be introduced to the RPM plugin API is the ability for a plugin hook to stop the plugin chain from being executed. Currently, every plugin that registers to a specific hook is called as part of the plugin chain, regardless of whatever action may have been performed by a previous plugin. In cases such as installing file or creating the rpmfi iterator, as soon as a plugin has performed that action, the chain must be interrupted. In the current code, this is done by the introduction of the RPMRC_PLUGIN_CONTENTS return code.

As mentioned previously, the final RPM which is on-disk after having being transcoded by rpm2extents is not a bitwise copy of the original RPM, as such signature/digest verifications on anything that includes the payload will not work.
The current approach is that rpm2extents performs the signature validation while streaming the original RPM. The result of this verification (return code + text output) is written into the transcoded RPM trailing metadata so it can be re-used later by rpmkeys, or anything that needs to validate the package like rpm -i. Having a plugin hook that can defer the validation of RPM to the plugin would help in separating that logic from RPM core.

[chantra]

cc @pmatilai @ffesti @malmond77 @davide125 @Conan-Kudo

[DemiMarie]

As mentioned previously, the final RPM which is on-disk after having being transcoded by rpm2extents is not a bitwise copy of the original RPM, as such signature/digest verifications on anything that includes the payload will not work.

A better approach would be to change the RPM format to provide a digest that can be verified in a streaming manner.

[chantra]

> As mentioned previously, the final RPM which is on-disk after having > being transcoded by rpm2extents is not a bitwise copy of the original RPM, > as such signature/digest verifications on anything that includes the > payload will not work. A better approach would be to change the RPM format to provide a digest that can be verified in a streaming manner.

The problem is not about verifying in a streaming manner, this is what is currently done. The problem is that the final transcoded rpm (after it has been verified) will not be able to pass `rpmkeys` anymore because it is not a bit wise copy of the original rpm anymore. I think what you mean by changing the rpm format is so that we can verify the rpm in chunks while streaming it vs in its integrity, this would be a different discussion and does not change the fact that once we have the resulting transcoded rpm, it will not pass the original signature verification because that rpm has now changed (for the purpose of illustration let say this is equivalent of having the original rpm with its payload compressed and storing it with its payload uncompressed, content of files that would eventually make it to the disk are the same, but the rpm is not a bit wise copy of the one we downloaded from the repo).

[DemiMarie]

I think what you mean by changing the rpm format is so that we can verify
the rpm in chunks while streaming it

That is exactly what I mean.

[chantra]

@pmatilai do you have any feedback on this?

[chantra]

@pmatilai, checking back if you have any feedback on this? Is there anything you need more details on?

[pmatilai]

Another new concept that needs to be introduced to the RPM plugin API is the ability for a plugin hook to stop the plugin chain from being executed. Currently, every plugin that registers to a specific hook is called as part of the plugin chain, regardless of whatever action may have been performed by a previous plugin. In cases such as installing file or creating the rpmfi iterator, as soon as a plugin has performed that action, the chain must be interrupted. In the current code, this is done by the introduction of the RPMRC_PLUGIN_CONTENTS return code.

I already said this in the original PR but maybe it got lost in the noise: this needs a different kind of API, interrupted chains is not a supportable concept. Something that allows the plugin to register as the owner of this payload format and the handler(s) called at appropriate times, with appropriate data. Such as the package file descriptor, and such as to actually install the file by reflinking it - AFAICS fsmUnpack(), or actually rpmfiArchiveReadToFilePsm() is the precise thing you'd want to replace with your own: you get a writable fd and you get to populate it with content by whatever means, while letting rpm handle all the other logic. That's about the extent of control I'm willing to consider handling to plugins.

So... what it all really comes down to is ability to define a custom archive type (see rpmfiNewArchiveReader()), and register them so that rpm knows to call it from fsmIter() in place of the internal default.

[pmatilai]

The reason this feels like trying to put a square peg through a round hole is that it's exactly what it is. The current plugin API was never intended to handle such a use-case, and so eg that RPMRC_PLUGIN_CONTENT is just trying to file off the corners a bit to make it somehow squeeze through 😅

This probably calls for a new plugin type to be introduced, one that specifically handles content only.

[chantra]

Thanks @pmatilai , this is really useful feedback.

I think I see how that could work. I will try to move to this model and report back as I hit roadblocks :)

[ddiss]

Thanks for working on this - the new CoW extent based approach looks exciting. I have a few comments / questions on the initial proposal...

Files are converted (“transcoded”) locally during download using /usr/bin/rpm2extents (part of rpm codebase). The format is not intended to be “portable” - i.e. copying the files from the cache is not supported.

Regular RPMs use a compressed .cpio based payload. In contrast, extent based RPMs contain uncompressed data aligned to the fundamental page size of the architecture, e.g. 4KiB on x86_64. This alignment is required for FICLONERANGE to work. Only files are represented in the payload, other directory entries like symlinks, device nodes etc are constructed entirely from rpm header information. Files are referenced by their digest, so identical files are de-duplicated.

I just wanted to highlight that, although not optimal, cpio archives can still be used alongside reflinks if the newc spec is bent a little to provide file data-segment alignment. The kernel initramfs archive format is quite static, so for dracut-cpio (dracut --enhanced-cpio) reflinks I added functionality to zero-pad file names so that subsequent file data segments are block aligned. kernel and GNU cpio extractors handle this fine, with the caveat that padding can't exceed PATH_MAX.

On a separate note, Btrfs recently gained support for writing compressed extents directly to disk via the new BTRFS_IOC_ENCODED_WRITE ioctl. IIUC, Fedora also recently changed to using zstd by default for Btrfs root and rpms. Have you considered using BTRFS_IOC_ENCODED_WRITE functionality instead of performing decompression during download?

[ddiss]

I think as long as the zstd frame boundaries align with individual file data segments then it should still work fine with BTRFS_IOC_ENCODED_WRITE and reflink, although we'd need to write out individual files immediately rather than staging the complete cpio archive.

[DemiMarie]

@ddiss: From my perspective, plugins should not deal with untrusted input, so “used” means “passed to the plugin”. This means that data must be buffered in memory until it can be cryptographically verified. This is not possible with the current digest format, but a new format can support very efficient streaming verification, requiring only O(log N) or even O(1) space.

A simple design is something like this:

struct signed_block {
    uint16_t size; // size of this block, limited to 64KiB
                   // but must be at least 1KiB (except last block)
    char hash[64]; // Hash of the next block, or all zeros for EOF
    char payload[]; // size - 64 bytes
};

With this proposal, the payload digest only covers the first block in the payload. Once the first block is hashed, the payload is released to the plugin, while the hash is stored by RPM to verify the next block.

Another option is to use per-block hashes stored in the RPM header. This bloats the RPM header, but allows for hash generation and verification to happen in parallel. Assuming a 1MiB block size, a 1TiB RPM (which is absolutely massive), a 512-bit hash, and the use of a byte array tag in the header (more efficient than a string array), this would require ((2⁴⁰ ÷ 2²⁰) × (512 ÷ 8)) = 64MiB of space in the header, which is well within the limit of 256MiB.

Whichever solution is chosen, RPM should provide a way to calculate a hash of the package that includes everything except the payload, and which returns an error if there is no payload digest. This can be included in metadata.

[ddiss]

@DemiMarie I wonder whether packaging fsverity (Merkle tree) metadata into the rpm header would be an option for performant block-based hashing. It'd also bloat the rpm header, but may have the benefit of allowing the metadata to be reused for post-installation integrity and authenticity protection (assuming the files remain read-only and reside on ext4/btrfs).
For my specific BTRFS_IOC_ENCODED_WRITE use case, I didn't intend to implement it as a plug-in, but as part of core. The implementation doesn't require modifications to the existing on-disk format, and the only FS specific behaviour is the actual encoded write ioctl calls.

[DemiMarie]

@ddiss fsverity would also be suitable. If you go with this approach, I recommend also including the total length of the payload in the (signed) header, to avoid vulnerabilities where extra data somehow doesn’t get hashed.

[pmatilai]

v6 will have signed payload size information : 784bb90

[rphibel]

Hello @pmatilai,

I work at Meta with @chantra and @malmond77 . I am implementing the new API for the RPM CoW plugin based on your comment. This is what I have implemented so far:

I defined 2 new fields of type rpmPlugin (with associated getters and setters) in rpmte structure:

• customArchiveReader
• customFileInstaller

When a plugin wants to register as an archive reader for a package, it sets the field customArchiveReader in the psm_pre stage.
Similarly, if it wants to register as a file installer it sets the customFileInstaller field.

Then in fsm.c, if these fields are set, archive reading, file installation is deferred to the plugin.

Here is the code:
rphibel/rpm@cow_denylist_rebased...cow_denylist_refactoring

Could you please let me know if this is what you had in mind?

[rphibel]

@pmatilai Hello, I was wondering if you had a chance to have a look at my proposal? Please let me know if this is unclear or if you need more details.

[pmatilai]

Okay so, trying to give a better idea of what I'm after:

The fact that RPMRC_PLUGIN_CONTENTS is still needed is a strong signal that it's not quite right.
The way I'm imagining this is that you have a probe function for the content handler, that gets called exactly one for each and every package, early on in every single transaction,. For stock rpm content, it just returns pointers to internal functions (or perhaps a struct with these pointers). For other content, it returns the handler from the first plugin that claims the content type and if nothing handles it, it's obviously an error. This should happen at the stage where the transaction is populated, not during it.

The fsm code for handling different types doesn't differ at all, it's just a function pointer that gets called. Ie there's no rpmpluginsCallFsmFileArchiveReader() or rpmpluginsCallFsmFilePrepare(), these are determined by the early probe. And so there are no rpmteSetCustomFileInstaller() or rpmteSetCustomArchiveReader() or any corresponding custom-call-functions either, it's always just a function pointer to whatever is handling the content.

[DemiMarie]

To add to this: Plugins should not get access to content that has not been verified yet. That means creating a new method of cryptographic verification, one that allows streaming verification of the data.

[rphibel]

Thanks a lot for these comments. I think I have a better idea of what I need to do. I will work on a new implementation.

[pmatilai]

To add to this: Plugins should not get access to content that has not been verified yet.

Regardless of the technical requirements, this is a fine example of what a sane API looks like: it's not about just making X somehow possible, but building the right usage into the API. Often easier said than done of course.

[ddiss]

Thing is, since the payload is typically compressed, offsets are useless because they'd just point to somewhere in the middle of a compression stream, you can't jump to it without reading the whole thing anyhow.

zstd frame headers can carry both compressed and uncompressed sizes, which makes it a little easier to seek around within the compressed payload.

If the files in the payload were individually compressed, it'd seem quite reasonable to have offsets stored. Of course that would likely loose something in the compression ratio.

That's mostly what I did in my rpm-rs based poc - files aren't individually compressed, but the start and end of individual file data segments correspond to zstd frame boundaries, so individual files can be extracted or written via BTRFS_IOC_ENCODED_WRITE/reflinked without much effort.

The rpm payload format isn't modified, although there's a slight "bending" of the cpio/newc spec to use the filename field for padding. zstandard frames making up the compressed rpm payload explicitly carry both compressed and uncompressed lengths, to allow detection of filesystem-supported I/O sizes.

Note that the cpio/newc format is on its way out, you don't want to design too much around that. In rpm v4 it's used for packages where all contained files are below 4GB, but v6 will always use the newer rpm-specific format which doesn't carry any file metadata in the payload: https://rpm-software-management.github.io/rpm/manual/format_v6.html#payload

Returning to this, I suppose I could still pad 07070X payloads relatively easily by injecting individual "padding" files into the archive. This would still be compatible with V4, although it might be nice to reserve a special padding path in the V6 spec which can be skipped over (where V4 would still extract / install it). Is the V6 draft still open for these kinds of proposals?
cc: @lnussel