Skip to content

Commit

Permalink
build: Add support for propagating RPM IMA signatures
Browse files Browse the repository at this point in the history
The alignment between ostree and IMA is actually quite good.  IMA
signatures are just extended attributes, and ostree has first-class
native support for those.

RPM represents IMA signatures in an ad-hoc way, and then a plugin
pulls bits from the header and does the `lsetxattr` call.

One thing that ostree is pretty good at is that one can synthesize
data in memory into the final ostree commit, without actually needing
it to hit the physical filesystem at build time.  This is crucial
for SELinux, where we want to be able to set security contexts
in the resulting build without actually requiring matching SELinux
policy on the *build* server.

IMA has similar issues; we don't want to blindly use `rpm-plugin-ima`
at *build* time.  That would potentially conflict with IMA policy
set up for the host system.

Add a treefile option `ima: true` that if set, causes the importer
to propagate the RPM IMA signatures into the proper `security.ima`
extended attribute.

Now, while working on this I made a fundamental realization that
all along, we should have supported committing `bare-user` checkouts
directly into `archive` repositories.

This is how it works in coreos-assembler today.  It'd make a ton
of sense to add something like `ostree commit --from-bare-user` that
would honor the `ostree.usermeta` xattr.  (Tempting to do it by default
but the compatibility hazard seems high)

So for now...hack in code in our commit phase to do this.

Note there was previous work here to automatically translate the
`user.ima` xattr to `security.ima` (because it was apparently easier
than patching rpm-ostree core?).  This code kind of obsoletes that,
but on the other hand one could today be writing `user.ima` attributes
during a build process for things that are not from RPM, and that's
kind of useful to continue to support.  Though, I think we should
handle that in a more principled way (that's a bigger topic).

Note that Fedora and CentOS do not ship native IMA signatures by
default.  And in fact, there was a *huge* mess around the representation
of IMA signatures in rpm - so patches to fix that landed in C9S/RHEL9
but not in at least Fedora 35 I believe.

Which means: actually using this functionality for e.g. RHEL9 IMA
signatures requires using rpm-ostree (and really, librpm) from
a RHEL9 host system - it won't work today to "cross build" as
we do with coreos-assembler.

Also, testing this out with RHEL9 content I needed to do:
```
remove-from-packages:
  - - systemd-libs
    - /usr/share/licenses/systemd/LICENSE.LGPL2.1
```

because there are two copies of that file, but they apparently
have separate IMA signatures and only when IMA is enabled, ostree
(correctly) reports them as conflicting.

Closes: coreos#3547
  • Loading branch information
cgwalters committed Apr 16, 2022
1 parent 9969932 commit 8615522
Show file tree
Hide file tree
Showing 8 changed files with 128 additions and 12 deletions.
3 changes: 3 additions & 0 deletions docs/treefile.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ It supports the following parameters:
* `selinux`: boolean, optional: Defaults to `true`. If `false`, then
no SELinux labeling will be performed on the server side.

* `ima`: boolean, optional: Defaults to `false`. Propagate any
IMA signatures in input RPMs into the final OSTree commit.

* `boot-location` (or `boot_location`): string, optional:
There are 2 possible values:
* "new": A misnomer, this value is no longer "new". Kernel data
Expand Down
1 change: 1 addition & 0 deletions rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,7 @@ pub mod ffi {
fn get_documentation(&self) -> bool;
fn get_recommends(&self) -> bool;
fn get_selinux(&self) -> bool;
fn get_ima(&self) -> bool;
fn get_releasever(&self) -> String;
fn get_repo_metadata_target(&self) -> RepoMetadataTarget;
fn rpmdb_backend_is_target(&self) -> bool;
Expand Down
7 changes: 7 additions & 0 deletions rust/src/treefile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,7 @@ fn treefile_merge(dest: &mut TreeComposeConfig, src: &mut TreeComposeConfig) {
basearch,
rojig,
selinux,
ima,
gpg_key,
include,
container,
Expand Down Expand Up @@ -993,6 +994,10 @@ impl Treefile {
self.parsed.base.selinux.unwrap_or(true)
}

pub(crate) fn get_ima(&self) -> bool {
self.parsed.base.ima.unwrap_or(false)
}

pub(crate) fn get_releasever(&self) -> String {
self.parsed
.base
Expand Down Expand Up @@ -2003,6 +2008,8 @@ pub(crate) struct BaseComposeConfigFields {
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) selinux: Option<bool>,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) ima: Option<bool>,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) gpg_key: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) include: Option<Include>,
Expand Down
3 changes: 3 additions & 0 deletions src/libpriv/rpmostree-core.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -2363,6 +2363,9 @@ start_async_import_one_package (RpmOstreeContext *self, DnfPackage *pkg, GCancel
if (self->treefile_rs->get_readonly_executables ())
flags |= RPMOSTREE_IMPORTER_FLAGS_RO_EXECUTABLES;

if (self->treefile_rs->get_ima ())
flags |= RPMOSTREE_IMPORTER_FLAGS_IMA;

/* TODO - tweak the unpacker flags for containers */
OstreeRepo *ostreerepo = get_pkgcache_repo (self);
g_autoptr (RpmOstreeImporter) unpacker = rpmostree_importer_new_take_fd (
Expand Down
4 changes: 4 additions & 0 deletions src/libpriv/rpmostree-core.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ G_BEGIN_DECLS
#define RPMOSTREE_DIR_CACHE_SOLV "solv"
#define RPMOSTREE_DIR_LOCK "lock"

// Extended attribute used at build time.
#define RPMOSTREE_USER_IMA "user.ima"
#define RPMOSTREE_SYSTEM_IMA "security.ima"

/* See http://lists.rpm.org/pipermail/rpm-maint/2017-October/006681.html */
/* This is also defined on the Rust side. */
#define RPMOSTREE_RPMDB_LOCATION "usr/share/rpm"
Expand Down
70 changes: 63 additions & 7 deletions src/libpriv/rpmostree-importer.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include <grp.h>
#include <pwd.h>
#include <rpm/rpmfi.h>
#include <rpm/rpmfiles.h>
#include <rpm/rpmlib.h>
#include <rpm/rpmlog.h>
#include <rpm/rpmts.h>
Expand Down Expand Up @@ -154,7 +155,7 @@ rpmostree_importer_read_metainfo (int fd, Header *out_header, gsize *out_cpio_of

if (out_fi)
{
ret_fi = rpmfiNew (ts, ret_header, RPMTAG_BASENAMES, (RPMFI_NOHEADER | RPMFI_FLAGS_INSTALL));
ret_fi = rpmfiNew (ts, ret_header, RPMTAG_BASENAMES, RPMFI_NOHEADER | RPMFI_FLAGS_QUERY);
ret_fi = rpmfiInit (ret_fi, 0);
}

Expand All @@ -167,6 +168,30 @@ rpmostree_importer_read_metainfo (int fd, Header *out_header, gsize *out_cpio_of
return TRUE;
}

/*
* ima_heck_zero_hdr: Check the signature for a zero header
*
* Check whether the given signature has a header with all zeros
*
* Returns -1 in case the signature is too short to compare
* (invalid signature), 0 in case the header is not only zeroes,
* and 1 if it is only zeroes.
*/
static int
ima_check_zero_hdr (const unsigned char *fsig, size_t siglen)
{
/*
* Every signature has a header signature_v2_hdr as defined in
* Linux's (4.5) security/integrity/integtrity.h. The following
* 9 bytes represent this header in front of the signature.
*/
static const uint8_t zero_hdr[] = { 0, 0, 0, 0, 0, 0, 0, 0, 0 };

if (siglen < sizeof (zero_hdr))
return -1;
return (memcmp (fsig, &zero_hdr, sizeof (zero_hdr)) == 0);
}

static void
build_rpmfi_overrides (RpmOstreeImporter *self)
{
Expand All @@ -187,10 +212,14 @@ build_rpmfi_overrides (RpmOstreeImporter *self)
const char *fn = rpmfiFN (self->fi);
rpmfileAttrs fattrs = rpmfiFFlags (self->fi);

size_t siglen = 0;
const unsigned char *fsig = rpmfiFSignature (self->fi, &siglen);
const bool have_ima = (siglen > 0 && fsig && (ima_check_zero_hdr (fsig, siglen) == 0));

const gboolean user_is_root = (user == NULL || g_str_equal (user, "root"));
const gboolean group_is_root = (group == NULL || g_str_equal (group, "root"));
const gboolean fcaps_is_unset = (fcaps == NULL || fcaps[0] == '\0');
if (!(user_is_root && group_is_root && fcaps_is_unset))
if (!(user_is_root && group_is_root && fcaps_is_unset) || have_ima)
{
g_hash_table_insert (self->rpmfi_overrides, g_strdup (fn), GINT_TO_POINTER (i));
}
Expand Down Expand Up @@ -253,7 +282,7 @@ rpmostree_importer_new_take_fd (int *fd, OstreeRepo *repo, DnfPackage *pkg,

static void
get_rpmfi_override (RpmOstreeImporter *self, const char *path, const char **out_user,
const char **out_group, const char **out_fcaps)
const char **out_group, const char **out_fcaps, GVariant **out_ima)
{
gpointer v;

Expand All @@ -270,6 +299,18 @@ get_rpmfi_override (RpmOstreeImporter *self, const char *path, const char **out_
*out_group = rpmfiFGroup (self->fi);
if (out_fcaps)
*out_fcaps = rpmfiFCaps (self->fi);

if (out_ima)
{
size_t siglen;
const guint8 *fsig = rpmfiFSignature (self->fi, &siglen);
if (siglen > 0 && fsig && (ima_check_zero_hdr (fsig, siglen) == 0))
{
GVariant *ima_signature
= g_variant_new_fixed_array (G_VARIANT_TYPE_BYTE, fsig, siglen, 1);
*out_ima = g_variant_ref_sink (ima_signature);
}
}
}

const char *
Expand Down Expand Up @@ -471,7 +512,7 @@ compose_filter_cb (OstreeRepo *repo, const char *path, GFileInfo *file_info, gpo
/* Lookup any rpmfi overrides (was parsed from the header) */
const char *user = NULL;
const char *group = NULL;
get_rpmfi_override (self, path, &user, &group, NULL);
get_rpmfi_override (self, path, &user, &group, NULL, NULL);

auto entry = ROSCXX_VAL (
tmpfiles_translate (path, *file_info, user ?: "root", group ?: "root"), error);
Expand Down Expand Up @@ -511,12 +552,27 @@ xattr_cb (OstreeRepo *repo, const char *path, GFileInfo *file_info, gpointer use
auto self = static_cast<RpmOstreeImporter *> (user_data);
const char *fcaps = NULL;

get_rpmfi_override (self, path, NULL, NULL, &fcaps);
GVariant *imasig = NULL;
const bool use_ima = self->flags & RPMOSTREE_IMPORTER_FLAGS_IMA;
get_rpmfi_override (self, path, NULL, NULL, &fcaps, use_ima ? &imasig : NULL);

g_auto (GVariantBuilder) builder = {
0,
};
g_variant_builder_init (&builder, (GVariantType *)"a(ayay)");
if (fcaps != NULL && fcaps[0] != '\0')
return rpmostree_fcap_to_xattr_variant (fcaps);
{
g_autoptr (GVariant) fcaps_v = rpmostree_fcap_to_xattr_variant (fcaps);
g_variant_builder_add_value (&builder, fcaps_v);
}

if (imasig)
{
g_variant_builder_add (&builder, "(@ay@ay)", g_variant_new_bytestring (RPMOSTREE_SYSTEM_IMA),
imasig);
}

return NULL;
return g_variant_ref_sink (g_variant_builder_end (&builder));
}

static char *
Expand Down
2 changes: 2 additions & 0 deletions src/libpriv/rpmostree-importer.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,14 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC (RpmOstreeImporter, g_object_unref)
* ostree-compliant paths rather than erroring out
* @RPMOSTREE_IMPORTER_FLAGS_NODOCS: Skip documentation files
* @RPMOSTREE_IMPORTER_FLAGS_RO_EXECUTABLES: Make executable files readonly
* @RPMOSTREE_IMPORTER_FLAGS_IMA: Enable IMA
*/
typedef enum
{
RPMOSTREE_IMPORTER_FLAGS_SKIP_EXTRANEOUS = (1 << 0),
RPMOSTREE_IMPORTER_FLAGS_NODOCS = (1 << 1),
RPMOSTREE_IMPORTER_FLAGS_RO_EXECUTABLES = (1 << 2),
RPMOSTREE_IMPORTER_FLAGS_IMA = (1 << 3),
} RpmOstreeImporterFlags;

RpmOstreeImporter *rpmostree_importer_new_take_fd (int *fd, OstreeRepo *repo, DnfPackage *pkg,
Expand Down
50 changes: 45 additions & 5 deletions src/libpriv/rpmostree-postprocess.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,30 @@ struct CommitThreadData
GError **error;
};

// In unified core mode, we'll see user-mode checkout files.
// What we want for now is to access the embedded xattr values
// which (along with other canonical file metadata) have been serialized
// into the `user.ostreemeta` extended attribute. What we should
// actually do is add nice support for this in core ostree, and also
// automatically pick up file owner for example. This would be a key
// thing to unblock fully unprivileged builds.
//
// But for now, just slurp up the xattrs so we get IMA in particular.
static void
extend_ostree_xattrs (GVariantBuilder *builder, GVariant *vbytes)
{
g_autoptr (GBytes) bytes = g_variant_get_data_as_bytes (vbytes);
g_autoptr (GVariant) filemeta = g_variant_ref_sink (
g_variant_new_from_bytes (OSTREE_FILEMETA_GVARIANT_FORMAT, bytes, false));
g_autoptr (GVariant) xattrs = g_variant_get_child_value (filemeta, 3);
g_autoptr (GVariantIter) viter = g_variant_iter_new (xattrs);
GVariant *key, *value;
while (g_variant_iter_loop (viter, "(@ay@ay)", &key, &value))
{
g_variant_builder_add (builder, "(@ay@ay)", key, value);
}
}

/* Filters out all xattrs that aren't accepted. */
static GVariant *
filter_xattrs_cb (OstreeRepo *repo, const char *relpath, GFileInfo *file_info, gpointer user_data)
Expand All @@ -662,7 +686,7 @@ filter_xattrs_cb (OstreeRepo *repo, const char *relpath, GFileInfo *file_info, g
static const char *accepted_xattrs[] = {
"security.capability", /* https://lwn.net/Articles/211883/ */
"user.pax.flags", /* https://github.com/projectatomic/rpm-ostree/issues/412 */
"user.ima" /* will be replaced with security.ima */
RPMOSTREE_USER_IMA, /* will be replaced with security.ima */
};
g_autoptr (GVariant) existing_xattrs = NULL;
g_autoptr (GVariantIter) viter = NULL;
Expand All @@ -671,6 +695,7 @@ filter_xattrs_cb (OstreeRepo *repo, const char *relpath, GFileInfo *file_info, g
GVariant *key, *value;
GVariantBuilder builder;

// From here, ensure the path is relative
if (relpath[0] == '/')
relpath++;

Expand All @@ -697,15 +722,30 @@ filter_xattrs_cb (OstreeRepo *repo, const char *relpath, GFileInfo *file_info, g

while (g_variant_iter_loop (viter, "(@ay@ay)", &key, &value))
{
const char *attrkey = g_variant_get_bytestring (key);

// If it's the special bare-user xattr, then slurp out the embedded
// xattrs.
if (g_str_equal (attrkey, "user.ostreemeta"))
{
extend_ostree_xattrs (&builder, value);
continue;
}

// Otherwise, process our allowlist of xattrs.
for (guint i = 0; i < G_N_ELEMENTS (accepted_xattrs); i++)
{
const char *validkey = accepted_xattrs[i];
const char *attrkey = g_variant_get_bytestring (key);
if (g_str_equal (validkey, attrkey))
{
if (g_str_equal (validkey, "user.ima"))
g_variant_builder_add (&builder, "(@ay@ay)",
g_variant_new_bytestring ("security.ima"), value);
// Translate user.ima to its final security.ima value. This allows handling
// IMA outside of rpm-ostree, without needing IMA to be enabled on the
// "host" system.
if (g_str_equal (validkey, RPMOSTREE_USER_IMA))
{
g_variant_builder_add (&builder, "(@ay@ay)",
g_variant_new_bytestring (RPMOSTREE_SYSTEM_IMA), value);
}
else
g_variant_builder_add (&builder, "(@ay@ay)", key, value);
}
Expand Down

0 comments on commit 8615522

Please sign in to comment.