From 8615522c962276b2bd060b44bae7abb63236a6e8 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 15 Apr 2022 11:42:28 -0400 Subject: [PATCH] build: Add support for propagating RPM IMA signatures 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: https://github.com/coreos/rpm-ostree/issues/3547 --- docs/treefile.md | 3 ++ rust/src/lib.rs | 1 + rust/src/treefile.rs | 7 +++ src/libpriv/rpmostree-core.cxx | 3 ++ src/libpriv/rpmostree-core.h | 4 ++ src/libpriv/rpmostree-importer.cxx | 70 ++++++++++++++++++++++++--- src/libpriv/rpmostree-importer.h | 2 + src/libpriv/rpmostree-postprocess.cxx | 50 +++++++++++++++++-- 8 files changed, 128 insertions(+), 12 deletions(-) diff --git a/docs/treefile.md b/docs/treefile.md index 1fe8da9f4b..b3667a6983 100644 --- a/docs/treefile.md +++ b/docs/treefile.md @@ -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 diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 57f6e291be..3a0117af7c 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -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; diff --git a/rust/src/treefile.rs b/rust/src/treefile.rs index c65c70a108..cbc4e011ec 100644 --- a/rust/src/treefile.rs +++ b/rust/src/treefile.rs @@ -406,6 +406,7 @@ fn treefile_merge(dest: &mut TreeComposeConfig, src: &mut TreeComposeConfig) { basearch, rojig, selinux, + ima, gpg_key, include, container, @@ -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 @@ -2003,6 +2008,8 @@ pub(crate) struct BaseComposeConfigFields { #[serde(skip_serializing_if = "Option::is_none")] pub(crate) selinux: Option, #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) ima: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub(crate) gpg_key: Option, #[serde(skip_serializing_if = "Option::is_none")] pub(crate) include: Option, diff --git a/src/libpriv/rpmostree-core.cxx b/src/libpriv/rpmostree-core.cxx index 2ed9c36732..b84522121b 100644 --- a/src/libpriv/rpmostree-core.cxx +++ b/src/libpriv/rpmostree-core.cxx @@ -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 ( diff --git a/src/libpriv/rpmostree-core.h b/src/libpriv/rpmostree-core.h index 058192b159..a4482f4234 100644 --- a/src/libpriv/rpmostree-core.h +++ b/src/libpriv/rpmostree-core.h @@ -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" diff --git a/src/libpriv/rpmostree-importer.cxx b/src/libpriv/rpmostree-importer.cxx index fbafb56e81..7d4f5a2ccd 100644 --- a/src/libpriv/rpmostree-importer.cxx +++ b/src/libpriv/rpmostree-importer.cxx @@ -40,6 +40,7 @@ #include #include #include +#include #include #include #include @@ -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); } @@ -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) { @@ -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)); } @@ -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; @@ -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 * @@ -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); @@ -511,12 +552,27 @@ xattr_cb (OstreeRepo *repo, const char *path, GFileInfo *file_info, gpointer use auto self = static_cast (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 * diff --git a/src/libpriv/rpmostree-importer.h b/src/libpriv/rpmostree-importer.h index 980c1946c4..ad0577af71 100644 --- a/src/libpriv/rpmostree-importer.h +++ b/src/libpriv/rpmostree-importer.h @@ -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, diff --git a/src/libpriv/rpmostree-postprocess.cxx b/src/libpriv/rpmostree-postprocess.cxx index f7b853bb11..57441c1d9d 100644 --- a/src/libpriv/rpmostree-postprocess.cxx +++ b/src/libpriv/rpmostree-postprocess.cxx @@ -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) @@ -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; @@ -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++; @@ -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); }