Skip to content

Commit

Permalink
wip: look up selinux label as we unpack into tree
Browse files Browse the repository at this point in the history
  • Loading branch information
jlebon committed Mar 31, 2016
1 parent 37998a7 commit 98b6747
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 25 deletions.
11 changes: 11 additions & 0 deletions src/daemon/rpmostree-sysroot-upgrader.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "rpmostree-sysroot-upgrader.h"
#include "rpmostree-core.h"
#include "rpmostree-rpm-util.h"
#include "rpmostree-postprocess.h"

#include "ostree-repo.h"

Expand Down Expand Up @@ -931,6 +932,16 @@ overlay_final_pkgset (RpmOstreeSysrootUpgrader *self,
hif_context_set_repo_dir (hifctx, reposdir);
}

/* load the sepolicy to use during import */
{
glnx_unref_object OstreeSePolicy *sepolicy = NULL;
if (!rpmostree_prepare_rootfs_get_sepolicy (tmprootfs_dfd, ".", &sepolicy,
cancellable, error))
goto out;

rpmostree_context_set_sepolicy (ctx, sepolicy);
}

/* --- Downloading metadata --- */
if (!rpmostree_context_download_metadata (ctx, cancellable, error))
goto out;
Expand Down
34 changes: 24 additions & 10 deletions src/libpriv/rpmostree-core.c
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ struct _RpmOstreeContext {
HifContext *hifctx;
OstreeRepo *ostreerepo;
char *dummy_instroot_path;
OstreeSePolicy *sepolicy;
};

G_DEFINE_TYPE (RpmOstreeContext, rpmostree_context, G_TYPE_OBJECT)
Expand Down Expand Up @@ -91,6 +92,8 @@ rpmostree_context_finalize (GObject *object)

g_clear_object (&rctx->ostreerepo);

g_clear_object (&rctx->sepolicy);

G_OBJECT_CLASS (rpmostree_context_parent_class)->finalize (object);
}

Expand Down Expand Up @@ -254,11 +257,21 @@ rpmostree_context_new_unprivileged (int userroot_dfd,
/* XXX: or put this in new_system() instead? */
void
rpmostree_context_set_repo (RpmOstreeContext *self,
OstreeRepo *repo)
OstreeRepo *repo)
{
g_set_object (&self->ostreerepo, repo);
}

/* I debated making this part of the treespec. Overall, I think it makes more
* sense to define it outside since the policy to use depends on the context in
* which the RpmOstreeContext is used, not something we can always guess on our
* own correctly. */
void
rpmostree_context_set_sepolicy (RpmOstreeContext *self,
OstreeSePolicy *sepolicy)
{
g_set_object (&self->sepolicy, sepolicy);
}

HifContext *
rpmostree_context_get_hif (RpmOstreeContext *self)
Expand Down Expand Up @@ -1096,12 +1109,13 @@ rpmostree_context_download_rpms (RpmOstreeContext *ctx,
}

static gboolean
import_one_package (OstreeRepo *ostreerepo,
int tmpdir_dfd,
HifContext *hifctx,
HifPackage * pkg,
GCancellable *cancellable,
GError **error)
import_one_package (OstreeRepo *ostreerepo,
int tmpdir_dfd,
HifContext *hifctx,
HifPackage *pkg,
OstreeSePolicy *sepolicy,
GCancellable *cancellable,
GError **error)
{
gboolean ret = FALSE;
g_autofree char *ostree_commit = NULL;
Expand All @@ -1123,8 +1137,8 @@ import_one_package (OstreeRepo *ostreerepo,
if (!unpacker)
goto out;

if (!rpmostree_unpacker_unpack_to_ostree (unpacker, ostreerepo, NULL, &ostree_commit,
cancellable, error))
if (!rpmostree_unpacker_unpack_to_ostree (unpacker, ostreerepo, sepolicy,
&ostree_commit, cancellable, error))
{
g_autofree char *nevra = hif_package_get_nevra (pkg);
g_prefix_error (error, "Unpacking %s: ", nevra);
Expand Down Expand Up @@ -1206,7 +1220,7 @@ rpmostree_context_download_import (RpmOstreeContext *self,
{
HifPackage *pkg = install->packages_to_download->pdata[i];
if (!import_one_package (self->ostreerepo, pkg_tempdir_dfd, hifctx, pkg,
cancellable, error))
self->sepolicy, cancellable, error))
goto out;
hif_state_assert_done (hifstate);
}
Expand Down
4 changes: 3 additions & 1 deletion src/libpriv/rpmostree-core.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ gboolean rpmostree_context_setup (RpmOstreeContext *self,
GError **error);

void rpmostree_context_set_repo (RpmOstreeContext *self,
OstreeRepo *repo);
OstreeRepo *repo);
void rpmostree_context_set_sepolicy (RpmOstreeContext *self,
OstreeSePolicy *sepolicy);

void rpmostree_hif_add_checksum_goal (GChecksum *checksum, HyGoal goal);
char *rpmostree_context_get_state_sha512 (RpmOstreeContext *self);
Expand Down
2 changes: 1 addition & 1 deletion src/libpriv/rpmostree-postprocess.c
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,7 @@ workaround_selinux_cross_labeling_recurse (int dfd,
return ret;
}

static gboolean
gboolean
rpmostree_prepare_rootfs_get_sepolicy (int dfd,
const char *path,
OstreeSePolicy **out_sepolicy,
Expand Down
7 changes: 7 additions & 0 deletions src/libpriv/rpmostree-postprocess.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@ rpmostree_rootfs_postprocess_common (int rootfs_fd,
GCancellable *cancellable,
GError **error);

gboolean
rpmostree_prepare_rootfs_get_sepolicy (int dfd,
const char *path,
OstreeSePolicy **out_sepolicy,
GCancellable *cancellable,
GError **error);

gboolean
rpmostree_prepare_rootfs_for_commit (GFile *rootfs,
JsonObject *treefile,
Expand Down
50 changes: 37 additions & 13 deletions src/libpriv/rpmostree-unpacker.c
Original file line number Diff line number Diff line change
Expand Up @@ -711,15 +711,34 @@ import_one_libarchive_entry_to_ostree (RpmOstreeUnpacker *self,

cap_t_to_vfs (caps, &vfscap, &vfscap_size);
vfsbytes = g_bytes_new (&vfscap, vfscap_size);

g_variant_builder_add (&xattr_builder, "(@ay@ay)",
g_variant_new_bytestring ("security.capability"),
g_variant_new_from_bytes ((GVariantType*)"ay",
vfsbytes,
FALSE));
FALSE));
}
}

// fetch the selinux label
if (sepolicy)
{
g_autofree char *label = NULL;
g_autoptr(GFileInfo) file_info =
_rpmostree_libarchive_to_file_info (entry);

This comment has been minimized.

Copy link
@cgwalters

cgwalters Apr 1, 2016

Can't we just get archive_entry_stat (entry)->st_mode ?

This comment has been minimized.

Copy link
@jlebon

jlebon Apr 1, 2016

Author Owner

Ahhh yes, that's better.

g_autofree char *fullpath = g_strdup_printf ("/%s", pathname);

This comment has been minimized.

Copy link
@cgwalters

cgwalters Apr 1, 2016

Hm, would be nice to avoid a malloc just for this but...oh well for now.


if (!ostree_sepolicy_get_label (sepolicy, fullpath,
g_file_info_get_attribute_uint32 (file_info, "unix::mode"),
&label, cancellable, error))
goto out;

if (label)
g_variant_builder_add (&xattr_builder, "(@ay@ay)",
g_variant_new_bytestring ("security.selinux"),
g_variant_new_bytestring (label));
}

if (!pathname[0])
{
parent = NULL;
Expand Down Expand Up @@ -917,7 +936,7 @@ rpmostree_unpacker_unpack_to_ostree (RpmOstreeUnpacker *self,
g_autoptr(GHashTable) rpmfi_overrides = NULL;
g_autoptr(GHashTable) hardlinks =
g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
g_autofree char *default_dir_checksum = NULL;
g_autofree char *default_dir_checksum = NULL;
g_autoptr(GFile) root = NULL;
glnx_unref_object OstreeMutableTree *mtree = NULL;
g_autoptr(GBytes) header_bytes = NULL;
Expand All @@ -928,17 +947,17 @@ rpmostree_unpacker_unpack_to_ostree (RpmOstreeUnpacker *self,

rpmfi_overrides = build_rpmfi_overrides (self);

g_assert (sepolicy == NULL);

/* Default directories are 0/0/0755, and right now we're ignoring
* SELinux. (This might be a problem for /etc, but in practice
* anything with nontrivial perms should be in the packages)
*/
/* Default directories are 0/0/0755. We're not ignoring SELinux, but we still
* need to create any parent dirs as they come since we don't have a proper
* rootfs tree. A better solution might be to just copy from e.g. the current
* deployment. In practice, this won't make a difference when we're overlaying
* over an existing rootfs.

This comment has been minimized.

Copy link
@cgwalters

cgwalters Apr 1, 2016

I think we're going to need to assign a label as well. We could arbitrarily pick the label for /usr ?

This comment has been minimized.

Copy link
@jlebon

jlebon Apr 1, 2016

Author Owner

Is this an issue in the unprivileged path? When I was testing it in a normal deployment, it seemed like the parent dirs all had their proper labels, I'm assuming because we're using OSTREE_REPO_CHECKOUT_OVERWRITE_UNION_FILES. Though I can see it being an issue for e.g. container assemble.

This comment has been minimized.

Copy link
@cgwalters

cgwalters Apr 1, 2016

It's likely because if we aren't specifying a label, the kernel will choose a default for us based on the parent label. It's the same reason why if you just invoke mkdir (or for that matter cat > foo.txt), you still end up with a security.selinux xattr.

This may actually be okay, but it would be one of the first cases where we have a diff in the deployment between the FS tree and the OSTree state.

This comment has been minimized.

Copy link
@jlebon

jlebon Apr 1, 2016

Author Owner

Oh I see, you're talking about dirs that won't even exist in the target deployment on which we'll later overlay. E.g. for /usr/bin, it wouldn't matter whether we assign a label, since it will already exist when we overlay with the proper label, but for something like /usr/libexec/myapp we would need to assign a label.

I think I'll just do it "properly" and look up the label for each individual parent dir, even if for most of them they'll already exist.

This comment has been minimized.

Copy link
@cgwalters

cgwalters Apr 1, 2016

Yeah. Looking up each dir is definitely the general solution. Will require some reworking of the code to create parent dirs I suspect.

* */
{ glnx_unref_object GFileInfo *default_dir_perms = g_file_info_new ();
g_file_info_set_attribute_uint32 (default_dir_perms, "unix::uid", 0);
g_file_info_set_attribute_uint32 (default_dir_perms, "unix::gid", 0);
g_file_info_set_attribute_uint32 (default_dir_perms, "unix::mode", 0755 | S_IFDIR);

if (!write_directory_meta (repo, default_dir_perms, NULL,
&default_dir_checksum, cancellable, error))
goto out;
Expand All @@ -951,18 +970,23 @@ rpmostree_unpacker_unpack_to_ostree (RpmOstreeUnpacker *self,
ostree_mutable_tree_set_metadata_checksum (mtree, default_dir_checksum);

{ g_autoptr(GBytes) metadata = NULL;

if (!get_lead_sig_header_as_bytes (self, &metadata, cancellable, error))
goto out;

g_variant_builder_add (&metadata_builder, "{sv}", "rpmostree.metadata",
g_variant_new_from_bytes ((GVariantType*)"ay", metadata, TRUE));
}


if (sepolicy)
g_variant_builder_add (&metadata_builder, "{sv}", "rpmostree.sepolicy",
g_variant_new_string
(ostree_sepolicy_get_csum (sepolicy)));

while (TRUE)
{
struct archive_entry *entry;

if (!next_archive_entry (self->archive, &entry, error))
goto out;
if (entry == NULL)
Expand Down

0 comments on commit 98b6747

Please sign in to comment.