From f007dbe6ef3639a41e2f9fb56a779d5d80ceaf73 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 28 Nov 2023 13:58:53 +0100 Subject: [PATCH] image: add new `ContainerBuildable` flag to OSTreeDiskImage One objective for bifrost images is that it should be possible to run osbuild inside a container. This can interfere with the selinux policies of the buildroot. Inside the container everything is labeled `system_u:object_r:container_files_t`. Labeling /usr/bin/osbuild as `osbuild_exec_t` is not possible in the general case because the host may not have `osbuild-selinux` installed that contains this type. The workaround is that the container labels osbuild itself as `install_exec_t`. This works fine however there is a selinux denial warning when the `{,u}mount` binary is called because the transition from `install_t`->`mount_t` is not allowed. The warning is "harmless" because `install_t` has enough privs to allow the `{,u}mount` binaries to work. To silence this warning we can label `{,u}mount` in the buildroot as `install_exec_t` directly. This commit allows to control this now via the `ContainerBuildable` flag that can be set on `manifest.Build` to enable this behavior. --- pkg/image/export_test.go | 15 +++++++ pkg/image/ostree_disk.go | 10 ++++- pkg/image/ostree_disk_test.go | 59 ++++++++++++++++++++++++ pkg/manifest/build.go | 9 ++++ pkg/manifest/build_test.go | 84 +++++++++++++++++++++++++++++++++++ 5 files changed, 176 insertions(+), 1 deletion(-) create mode 100644 pkg/image/export_test.go create mode 100644 pkg/image/ostree_disk_test.go create mode 100644 pkg/manifest/build_test.go diff --git a/pkg/image/export_test.go b/pkg/image/export_test.go new file mode 100644 index 0000000000..dfef80edc4 --- /dev/null +++ b/pkg/image/export_test.go @@ -0,0 +1,15 @@ +package image + +import ( + "github.com/osbuild/images/pkg/manifest" + "github.com/osbuild/images/pkg/rpmmd" + "github.com/osbuild/images/pkg/runner" +) + +func MockManifestNewBuild(new func(m *manifest.Manifest, runner runner.Runner, repos []rpmmd.RepoConfig) *manifest.Build) (restore func()) { + saved := manifestNewBuild + manifestNewBuild = new + return func() { + manifestNewBuild = saved + } +} diff --git a/pkg/image/ostree_disk.go b/pkg/image/ostree_disk.go index 19e8c105fd..59b31b4ea7 100644 --- a/pkg/image/ostree_disk.go +++ b/pkg/image/ostree_disk.go @@ -53,6 +53,10 @@ type OSTreeDiskImage struct { // Lock the root account in the deployment unless the user defined root // user options in the build configuration. LockRoot bool + + // Container buildable tweaks the buildroot to be container friendly, + // i.e. to not rely on an installed osbuild-selinux + ContainerBuildable bool } func NewOSTreeDiskImageFromCommit(commit ostree.SourceSpec) *OSTreeDiskImage { @@ -102,11 +106,15 @@ func baseRawOstreeImage(img *OSTreeDiskImage, m *manifest.Manifest, buildPipelin return manifest.NewRawOStreeImage(buildPipeline, osPipeline, img.Platform) } +// replaced in testing +var manifestNewBuild = manifest.NewBuild + func (img *OSTreeDiskImage) InstantiateManifest(m *manifest.Manifest, repos []rpmmd.RepoConfig, runner runner.Runner, rng *rand.Rand) (*artifact.Artifact, error) { - buildPipeline := manifest.NewBuild(m, runner, repos) + buildPipeline := manifestNewBuild(m, runner, repos) + buildPipeline.ContainerBuildable = img.ContainerBuildable buildPipeline.Checkpoint() // don't support compressing non-raw images diff --git a/pkg/image/ostree_disk_test.go b/pkg/image/ostree_disk_test.go new file mode 100644 index 0000000000..4f83ce50e9 --- /dev/null +++ b/pkg/image/ostree_disk_test.go @@ -0,0 +1,59 @@ +package image_test + +import ( + "math/rand" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/osbuild/images/internal/workload" + "github.com/osbuild/images/pkg/container" + "github.com/osbuild/images/pkg/image" + "github.com/osbuild/images/pkg/manifest" + "github.com/osbuild/images/pkg/platform" + "github.com/osbuild/images/pkg/rpmmd" + "github.com/osbuild/images/pkg/runner" +) + +func TestOSTreeDiskImageManifestSetsContainerBuildable(t *testing.T) { + rng := rand.New(rand.NewSource(0)) + + repos := []rpmmd.RepoConfig{} + r := &runner.Fedora{Version: 39} + + ref := "ostree/1/1/0" + containerSource := container.SourceSpec{ + Source: "source-spec", + Name: "name", + } + + var buildPipeline *manifest.Build + restore := image.MockManifestNewBuild(func(m *manifest.Manifest, r runner.Runner, repos []rpmmd.RepoConfig) *manifest.Build { + buildPipeline = manifest.NewBuild(m, r, repos) + return buildPipeline + }) + defer restore() + + for _, containerBuildable := range []bool{true, false} { + mf := manifest.New() + img := image.NewOSTreeDiskImageFromContainer(containerSource, ref) + require.NotNil(t, img) + img.Platform = &platform.X86{ + BasePlatform: platform.BasePlatform{ + ImageFormat: platform.FORMAT_QCOW2, + }, + BIOS: true, + UEFIVendor: "fedora", + } + img.Workload = &workload.BaseWorkload{} + img.OSName = "osname" + img.ContainerBuildable = containerBuildable + + _, err := img.InstantiateManifest(&mf, repos, r, rng) + require.Nil(t, err) + require.NotNil(t, img) + require.NotNil(t, buildPipeline) + + require.Equal(t, buildPipeline.ContainerBuildable, containerBuildable) + } +} diff --git a/pkg/manifest/build.go b/pkg/manifest/build.go index 65e5b6aa10..b0437252a7 100644 --- a/pkg/manifest/build.go +++ b/pkg/manifest/build.go @@ -22,6 +22,11 @@ type Build struct { dependents []Pipeline repos []rpmmd.RepoConfig packageSpecs []rpmmd.PackageSpec + + // TODO: make private? + // Container buildable tweaks the buildroot to be container friendly, + // i.e. to not rely on an installed osbuild-selinux + ContainerBuildable bool } // NewBuild creates a new build pipeline from the repositories in repos @@ -109,6 +114,10 @@ func (p *Build) getSELinuxLabels() map[string]string { switch pkg.Name { case "coreutils": labels["/usr/bin/cp"] = "system_u:object_r:install_exec_t:s0" + if p.ContainerBuildable { + labels["/usr/bin/mount"] = "system_u:object_r:install_exec_t:s0" + labels["/usr/bin/umount"] = "system_u:object_r:install_exec_t:s0" + } case "tar": labels["/usr/bin/tar"] = "system_u:object_r:install_exec_t:s0" } diff --git a/pkg/manifest/build_test.go b/pkg/manifest/build_test.go new file mode 100644 index 0000000000..5b74b071bc --- /dev/null +++ b/pkg/manifest/build_test.go @@ -0,0 +1,84 @@ +package manifest + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/osbuild/images/pkg/rpmmd" + "github.com/osbuild/images/pkg/runner" +) + +func TestBuildContainerBuildableNo(t *testing.T) { + repos := []rpmmd.RepoConfig{} + mf := New() + runner := &runner.Fedora{Version: 39} + + build := NewBuild(&mf, runner, repos) + require.NotNil(t, build) + + for _, tc := range []struct { + packageSpec []rpmmd.PackageSpec + containerBuildable bool + expectedSELinuxLabels map[string]string + }{ + // no pkgs means no selinux labels (container build or not) + { + []rpmmd.PackageSpec{}, + false, + map[string]string{}, + }, + { + []rpmmd.PackageSpec{}, + true, + map[string]string{}, + }, + { + []rpmmd.PackageSpec{{Name: "coreutils"}}, + false, + map[string]string{ + "/usr/bin/cp": "system_u:object_r:install_exec_t:s0", + }, + }, + { + []rpmmd.PackageSpec{{Name: "tar"}}, + false, + map[string]string{ + "/usr/bin/tar": "system_u:object_r:install_exec_t:s0", + }, + }, + { + []rpmmd.PackageSpec{{Name: "coreutils"}, {Name: "tar"}}, + false, + map[string]string{ + "/usr/bin/cp": "system_u:object_r:install_exec_t:s0", + "/usr/bin/tar": "system_u:object_r:install_exec_t:s0", + }, + }, + { + []rpmmd.PackageSpec{{Name: "coreutils"}}, + true, + map[string]string{ + "/usr/bin/cp": "system_u:object_r:install_exec_t:s0", + "/usr/bin/mount": "system_u:object_r:install_exec_t:s0", + "/usr/bin/umount": "system_u:object_r:install_exec_t:s0", + }, + }, + { + []rpmmd.PackageSpec{{Name: "coreutils"}, {Name: "tar"}}, + true, + map[string]string{ + "/usr/bin/cp": "system_u:object_r:install_exec_t:s0", + "/usr/bin/mount": "system_u:object_r:install_exec_t:s0", + "/usr/bin/umount": "system_u:object_r:install_exec_t:s0", + "/usr/bin/tar": "system_u:object_r:install_exec_t:s0", + }, + }, + } { + build.packageSpecs = tc.packageSpec + build.ContainerBuildable = tc.containerBuildable + + labels := build.getSELinuxLabels() + require.Equal(t, labels, tc.expectedSELinuxLabels) + } +}