From 48d638911196a37aeae5932c4bb1c7017271a806 Mon Sep 17 00:00:00 2001 From: Dusty Mabe Date: Tue, 17 Sep 2024 12:41:09 -0400 Subject: [PATCH 1/3] vmdeps: add reset and clear to supermin VM These are extremely useful when dealing with a limited serial console to try to restore some order to the output. --- src/vmdeps.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/vmdeps.txt b/src/vmdeps.txt index eaa9e95fad..2115988e0e 100644 --- a/src/vmdeps.txt +++ b/src/vmdeps.txt @@ -38,3 +38,7 @@ podman # For running osbuild osbuild osbuild-ostree osbuild-selinux osbuild-tools python3-pyrsistent + +# For resetting the terminal inside supermin shell +/usr/bin/reset +/usr/bin/clear From 51bb6ef167ee3747542dff52849c3776a74ec567 Mon Sep 17 00:00:00 2001 From: Dusty Mabe Date: Tue, 17 Sep 2024 12:42:35 -0400 Subject: [PATCH 2/3] workaround selinux issues with osbuild We have a few issues right now where files in our images don't have any selinux context (i.e. end up unlabeled_t). Here we workaround the hidden mountpoints issue [1] with a patch to OSBuild to hardcode some chcon calls. We workaround the "bunch of files under /sysroot are unlabeled" issue [2] by backported a proposed upstream change to the org.osbuild.selinux stage [3] and then using it to explicitly set the context on the root of the tree to `root_t`. We also add a fix [4] for another issue where '/boot/coreos/platforms.json' would end up with the wrong label. [1] https://github.com/coreos/fedora-coreos-tracker/issues/1771 [2] https://github.com/coreos/fedora-coreos-tracker/issues/1772 [3] https://github.com/osbuild/osbuild/pull/1889 [4] https://github.com/osbuild/osbuild/pull/1888 --- build.sh | 6 +- ...0001-hacks-for-coreos-selinux-issues.patch | 44 +++++++++++++ ...ages-coreos.platform-use-shutil.copy.patch | 31 +++++++++ ...on-t-require-file_contexts-if-labels.patch | 65 +++++++++++++++++++ .../coreos.osbuild.aarch64.mpp.yaml | 6 ++ .../coreos.osbuild.ppc64le.mpp.yaml | 6 ++ .../coreos.osbuild.s390x.mpp.yaml | 6 ++ .../coreos.osbuild.x86_64.mpp.yaml | 6 ++ 8 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 src/0001-hacks-for-coreos-selinux-issues.patch create mode 100644 src/0001-stages-coreos.platform-use-shutil.copy.patch create mode 100644 src/0001-stages-selinux-don-t-require-file_contexts-if-labels.patch diff --git a/build.sh b/build.sh index d4f28b67a1..e719d7d384 100755 --- a/build.sh +++ b/build.sh @@ -173,7 +173,11 @@ patch_osbuild() { mv /usr/bin/osbuild-mpp /usr/lib/osbuild/tools/ # Now all the software is under the /usr/lib/osbuild dir and we can patch - patch -d /usr/lib/osbuild -p1 < /usr/lib/coreos-assembler/0001-stages-dmverity-make-device-objects-more-generic.patch + cat /usr/lib/coreos-assembler/0001-stages-dmverity-make-device-objects-more-generic.patch \ + /usr/lib/coreos-assembler/0001-stages-coreos.platform-use-shutil.copy.patch \ + /usr/lib/coreos-assembler/0001-stages-selinux-don-t-require-file_contexts-if-labels.patch \ + /usr/lib/coreos-assembler/0001-hacks-for-coreos-selinux-issues.patch \ + | patch -d /usr/lib/osbuild -p1 # And then move the files back; supermin appliance creation will need it back # in the places delivered by the RPM. diff --git a/src/0001-hacks-for-coreos-selinux-issues.patch b/src/0001-hacks-for-coreos-selinux-issues.patch new file mode 100644 index 0000000000..449e365e5e --- /dev/null +++ b/src/0001-hacks-for-coreos-selinux-issues.patch @@ -0,0 +1,44 @@ +From 9faf7e2566cd9460ac51ff508c192bdc839ad2ef Mon Sep 17 00:00:00 2001 +From: Dusty Mabe +Date: Tue, 17 Sep 2024 12:27:37 -0400 +Subject: [PATCH 3/3] hacks for coreos selinux issues + +context in https://github.com/coreos/fedora-coreos-tracker/issues/1771#issuecomment-2348607969 +--- + osbuild/mounts.py | 13 ++++++++++++- + 1 file changed, 12 insertions(+), 1 deletion(-) + +diff --git a/osbuild/mounts.py b/osbuild/mounts.py +index 42b556ba..9b6c0804 100644 +--- a/osbuild/mounts.py ++++ b/osbuild/mounts.py +@@ -178,7 +178,12 @@ class FileSystemMountService(MountService): + + options = self.translate_options(options) + +- os.makedirs(mountpoint, exist_ok=True) ++ if not os.path.exists(mountpoint): ++ os.makedirs(mountpoint) ++ # Tactical fix for https://github.com/coreos/fedora-coreos-tracker/issues/1771 ++ if target == '/boot' or target == "/boot/efi": ++ subprocess.run(["chcon", "-v", "-t", 'boot_t', mountpoint], check=True) ++ + self.mountpoint = mountpoint + + print(f"mounting {source} -> {mountpoint}") +@@ -198,6 +203,12 @@ class FileSystemMountService(MountService): + msg = e.stdout.strip() + raise RuntimeError(f"{msg} (code: {code})") from e + ++ # Tactical fix for https://github.com/coreos/fedora-coreos-tracker/issues/1771 ++ # After the mount, let's make sure the lost+found directory has the right label ++ lostfounddir = os.path.join(mountpoint, 'lost+found') ++ if os.path.exists(lostfounddir): ++ subprocess.run(["chcon", "-v", "-t", 'lost_found_t', lostfounddir], check=True) ++ + self.check = True + return mountpoint + +-- +2.46.0 + diff --git a/src/0001-stages-coreos.platform-use-shutil.copy.patch b/src/0001-stages-coreos.platform-use-shutil.copy.patch new file mode 100644 index 0000000000..a516f52be7 --- /dev/null +++ b/src/0001-stages-coreos.platform-use-shutil.copy.patch @@ -0,0 +1,31 @@ +From 6b48c91e26efb448b2f2121b4179a1b79e15ce6d Mon Sep 17 00:00:00 2001 +From: Dusty Mabe +Date: Tue, 17 Sep 2024 12:18:45 -0400 +Subject: [PATCH 1/3] stages/coreos.platform: use shutil.copy + +Switch from shutil.copy2 so that we don't copy over the +SELinux labels from the source file. +--- + stages/org.osbuild.coreos.platform | 6 ++++-- + 1 file changed, 4 insertions(+), 2 deletions(-) + +diff --git a/stages/org.osbuild.coreos.platform b/stages/org.osbuild.coreos.platform +index a88951cc..7e66c26c 100755 +--- a/stages/org.osbuild.coreos.platform ++++ b/stages/org.osbuild.coreos.platform +@@ -52,8 +52,10 @@ def main(paths, options): + json_grub_args, json_kargs = None, None + if os.path.exists(platforms_source_path): + os.makedirs(os.path.dirname(platforms_dest_path), mode=0o755, exist_ok=True) +- # Copy platforms.json to the boot partition +- shutil.copy2(platforms_source_path, platforms_dest_path) ++ # Copy platforms.json to the boot partition. Use shutil.copy here and not ++ # shutil.copy2 because we don't want the selinux labels from the source ++ # to be copied over, but rather the defaults for the destination. ++ shutil.copy(platforms_source_path, platforms_dest_path) + json_grub_args, json_kargs = process_platforms_json(platforms_dest_path, platform) + if json_kargs: + kernel_arguments.extend(json_kargs) +-- +2.46.0 + diff --git a/src/0001-stages-selinux-don-t-require-file_contexts-if-labels.patch b/src/0001-stages-selinux-don-t-require-file_contexts-if-labels.patch new file mode 100644 index 0000000000..6cb8ffff6c --- /dev/null +++ b/src/0001-stages-selinux-don-t-require-file_contexts-if-labels.patch @@ -0,0 +1,65 @@ +From 281b0795fb4cc43ea05039627ebb5ff7130d70e9 Mon Sep 17 00:00:00 2001 +From: Dusty Mabe +Date: Tue, 17 Sep 2024 12:22:16 -0400 +Subject: [PATCH 2/3] stages/selinux: don't require file_contexts if labels + passed + +With the labels option the user is specifying the exact context +they want to set on the path so it's not necessary to supply a +context here. This can be also useful in the case where you want +to set some labels and you haven't yet populated the tree yet. +--- + stages/org.osbuild.selinux | 11 +++++++---- + stages/org.osbuild.selinux.meta.json | 13 +++++++++++-- + 2 files changed, 18 insertions(+), 6 deletions(-) + +diff --git a/stages/org.osbuild.selinux b/stages/org.osbuild.selinux +index bb45298d..563d827b 100755 +--- a/stages/org.osbuild.selinux ++++ b/stages/org.osbuild.selinux +@@ -8,11 +8,14 @@ from osbuild.util import selinux + + + def main(tree, options): +- file_contexts = os.path.join(f"{tree}", options["file_contexts"]) ++ file_contexts = options.get("file_contexts") + exclude_paths = options.get("exclude_paths") +- if exclude_paths: +- exclude_paths = [os.path.join(tree, p.lstrip("/")) for p in exclude_paths] +- selinux.setfiles(file_contexts, os.fspath(tree), "", exclude_paths=exclude_paths) ++ ++ if file_contexts: ++ file_contexts = os.path.join(f"{tree}", options["file_contexts"]) ++ if exclude_paths: ++ exclude_paths = [os.path.join(tree, p.lstrip("/")) for p in exclude_paths] ++ selinux.setfiles(file_contexts, os.fspath(tree), "", exclude_paths=exclude_paths) + + labels = options.get("labels", {}) + for path, label in labels.items(): +diff --git a/stages/org.osbuild.selinux.meta.json b/stages/org.osbuild.selinux.meta.json +index ea1bb3ef..151839e5 100644 +--- a/stages/org.osbuild.selinux.meta.json ++++ b/stages/org.osbuild.selinux.meta.json +@@ -20,8 +20,17 @@ + "schema_2": { + "options": { + "additionalProperties": false, +- "required": [ +- "file_contexts" ++ "oneOf": [ ++ { ++ "required": [ ++ "file_contexts" ++ ] ++ }, ++ { ++ "required": [ ++ "labels" ++ ] ++ } + ], + "properties": { + "file_contexts": { +-- +2.46.0 + diff --git a/src/osbuild-manifests/coreos.osbuild.aarch64.mpp.yaml b/src/osbuild-manifests/coreos.osbuild.aarch64.mpp.yaml index 7d40697e39..5404d7712b 100644 --- a/src/osbuild-manifests/coreos.osbuild.aarch64.mpp.yaml +++ b/src/osbuild-manifests/coreos.osbuild.aarch64.mpp.yaml @@ -139,6 +139,12 @@ pipelines: mpp-format-string: '{buildroot}' source-epoch: 1659397331 stages: + # Set the context of the root of the tree so that we avoid unlabeled_t files. + # https://github.com/coreos/fedora-coreos-tracker/issues/1772 + - type: org.osbuild.selinux + options: + labels: + /: system_u:object_r:root_t:s0 - type: org.osbuild.ostree.init-fs - type: org.osbuild.ostree.os-init options: diff --git a/src/osbuild-manifests/coreos.osbuild.ppc64le.mpp.yaml b/src/osbuild-manifests/coreos.osbuild.ppc64le.mpp.yaml index 3d7871dea3..ea5bbd7220 100644 --- a/src/osbuild-manifests/coreos.osbuild.ppc64le.mpp.yaml +++ b/src/osbuild-manifests/coreos.osbuild.ppc64le.mpp.yaml @@ -141,6 +141,12 @@ pipelines: mpp-format-string: '{buildroot}' source-epoch: 1659397331 stages: + # Set the context of the root of the tree so that we avoid unlabeled_t files. + # https://github.com/coreos/fedora-coreos-tracker/issues/1772 + - type: org.osbuild.selinux + options: + labels: + /: system_u:object_r:root_t:s0 - type: org.osbuild.ostree.init-fs - type: org.osbuild.ostree.os-init options: diff --git a/src/osbuild-manifests/coreos.osbuild.s390x.mpp.yaml b/src/osbuild-manifests/coreos.osbuild.s390x.mpp.yaml index 0e9ab31570..a12d53d345 100644 --- a/src/osbuild-manifests/coreos.osbuild.s390x.mpp.yaml +++ b/src/osbuild-manifests/coreos.osbuild.s390x.mpp.yaml @@ -169,6 +169,12 @@ pipelines: mpp-format-string: '{buildroot}' source-epoch: 1659397331 stages: + # Set the context of the root of the tree so that we avoid unlabeled_t files. + # https://github.com/coreos/fedora-coreos-tracker/issues/1772 + - type: org.osbuild.selinux + options: + labels: + /: system_u:object_r:root_t:s0 - type: org.osbuild.ostree.init-fs - type: org.osbuild.ostree.os-init options: diff --git a/src/osbuild-manifests/coreos.osbuild.x86_64.mpp.yaml b/src/osbuild-manifests/coreos.osbuild.x86_64.mpp.yaml index bb4f633a19..b473b499e3 100644 --- a/src/osbuild-manifests/coreos.osbuild.x86_64.mpp.yaml +++ b/src/osbuild-manifests/coreos.osbuild.x86_64.mpp.yaml @@ -141,6 +141,12 @@ pipelines: mpp-format-string: '{buildroot}' source-epoch: 1659397331 stages: + # Set the context of the root of the tree so that we avoid unlabeled_t files. + # https://github.com/coreos/fedora-coreos-tracker/issues/1772 + - type: org.osbuild.selinux + options: + labels: + /: system_u:object_r:root_t:s0 - type: org.osbuild.ostree.init-fs - type: org.osbuild.ostree.os-init options: From 8856e691c1a1ccbade2ea0a3ba91e3bb0bf8c2d3 Mon Sep 17 00:00:00 2001 From: Dusty Mabe Date: Fri, 20 Sep 2024 15:40:48 -0400 Subject: [PATCH 3/3] mantle: move kolet binary location to /usr/local/bin I'm writing a test that verifies files on the filesystem in CoreOS machinges match the SELinux policy. Placing kolet in `/var/home/core/kolet` with a `bin_t` context is a violation of this. Let's use /usr/local/bin/. This has the side effect of the file having the right `bin_t` context as soon as it is created. (cherry picked from commit b076a72f160edc9b605ef3728c6f96ded23c1c17) --- docs/kola/external-tests.md | 15 +++++++------- mantle/cmd/kolet/kolet.go | 4 ++-- mantle/kola/cluster/cluster.go | 2 +- mantle/kola/harness.go | 28 +++++++++++++++++++++----- mantle/kola/tests/ignition/resource.go | 2 +- mantle/kola/tests/ignition/security.go | 2 +- mantle/kola/tests/upgrade/basic.go | 4 ++-- 7 files changed, 37 insertions(+), 20 deletions(-) diff --git a/docs/kola/external-tests.md b/docs/kola/external-tests.md index 8d2b82d250..6b0b89ace1 100644 --- a/docs/kola/external-tests.md +++ b/docs/kola/external-tests.md @@ -112,11 +112,10 @@ method is deprecated and will be removed at some point) ## HTTP Server -The `kolet` binary is copied into the `core` user's home directory -(`/var/home/core`) on the CoreOS system running the tests. Notably, it contains -the built-in command `kolet httpd` for starting an HTTP file server to serve the -contents of the file system. -By default, it starts the server listening on port `80` and serves the contents of +The `kolet` binary is copied into the `/usr/local/bin/` directory on the CoreOS +system running the tests. Notably, it contains the built-in command `kolet httpd` +for starting an HTTP file server to serve the contents of the file system. By +default, it starts the server listening on port `80` and serves the contents of the file system at `./`; you can use the `--port` and `--path` flags to override the defaults. @@ -124,7 +123,7 @@ For example, if you're using a bash script as your test, you can start an HTTP server to serve the contents at `/var/home/core` like this: ``` echo testdata > /var/home/core/testdata.txt -systemd-run /var/home/core/kolet httpd --path /var/home/core/ +systemd-run /usr/local/bin/kolet httpd --path /var/home/core/ # It may take some time for the server to start. sleep 1 curl localhost/testdata.txt @@ -155,13 +154,13 @@ systemd: [Unit] Before=kola-runext.service [Path] - PathExists=/var/home/core/kolet + PathExists=/usr/local/bin/kolet [Install] WantedBy=kola-runext.service - name: kolet-httpd.service contents: | [Service] - ExecStart=/var/home/core/kolet httpd --path /var/www -v + ExecStart=/usr/local/bin/kolet httpd --path /var/www -v [Install] WantedBy=kola-runext.service storage: diff --git a/mantle/cmd/kolet/kolet.go b/mantle/cmd/kolet/kolet.go index d3b82d0f98..0093a225d7 100644 --- a/mantle/cmd/kolet/kolet.go +++ b/mantle/cmd/kolet/kolet.go @@ -97,14 +97,14 @@ const ( autopkgTestRebootPath = "/tmp/autopkgtest-reboot" autopkgtestRebootScript = `#!/bin/bash set -xeuo pipefail -~core/kolet reboot-request "$1" +/usr/local/bin/kolet reboot-request "$1" reboot ` autopkgTestRebootPreparePath = "/tmp/autopkgtest-reboot-prepare" autopkgtestRebootPrepareScript = `#!/bin/bash set -euo pipefail -exec ~core/kolet reboot-request "$1" +exec /usr/local/bin/kolet reboot-request "$1" ` // File used to communicate between the script and the kolet runner internally diff --git a/mantle/kola/cluster/cluster.go b/mantle/kola/cluster/cluster.go index 24f3bce037..3b9e133cf5 100644 --- a/mantle/kola/cluster/cluster.go +++ b/mantle/kola/cluster/cluster.go @@ -74,7 +74,7 @@ func (t *TestCluster) RunLogged(name string, f func(c TestCluster)) bool { // RunNative runs a registered NativeFunc on a remote machine func (t *TestCluster) RunNative(funcName string, m platform.Machine) bool { - command := fmt.Sprintf("./kolet run %q %q", t.H.Name(), funcName) + command := fmt.Sprintf("/usr/local/bin/kolet run %q %q", t.H.Name(), funcName) return t.Run(funcName, func(c TestCluster) { client, err := m.SSHClient() if err != nil { diff --git a/mantle/kola/harness.go b/mantle/kola/harness.go index 6ecd84b81d..ab2fdaa9d7 100644 --- a/mantle/kola/harness.go +++ b/mantle/kola/harness.go @@ -1118,10 +1118,10 @@ func runExternalTest(c cluster.TestCluster, mach platform.Machine, testNum int) // This is a non-exclusive test unit := fmt.Sprintf("%s-%d.service", KoletExtTestUnit, testNum) // Reboot requests are disabled for non-exclusive tests - cmd = fmt.Sprintf("sudo ./kolet run-test-unit --deny-reboots %s", shellquote.Join(unit)) + cmd = fmt.Sprintf("sudo /usr/local/bin/kolet run-test-unit --deny-reboots %s", shellquote.Join(unit)) } else { unit := fmt.Sprintf("%s.service", KoletExtTestUnit) - cmd = fmt.Sprintf("sudo ./kolet run-test-unit %s", shellquote.Join(unit)) + cmd = fmt.Sprintf("sudo /usr/local/bin/kolet run-test-unit %s", shellquote.Join(unit)) } stdout, err = c.SSH(mach, cmd) @@ -1893,9 +1893,14 @@ func runTest(h *harness.H, t *register.Test, pltfrm string, flight platform.Flig t.Run(tcluster) } -// ScpKolet searches for a kolet binary and copies it to the machine. +// ScpKolet searches for a kolet binary and copies it to the machines. +// Write initially to a .partial file in the same directory and then +// rename since systemd.path units may be watching and we don't want +// them to start while the file is still transferring. func ScpKolet(machines []platform.Machine) error { mArch := Options.CosaBuildArch + remotepath := "/usr/local/bin/kolet" + remotepathpartial := remotepath + ".partial" exePath, err := os.Executable() if err != nil { return errors.Wrapf(err, "finding path of executable") @@ -1908,8 +1913,21 @@ func ScpKolet(machines []platform.Machine) error { } { kolet := filepath.Join(d, "kolet") if _, err := os.Stat(kolet); err == nil { - if err := cluster.DropLabeledFile(machines, kolet, "bin_t"); err != nil { - return errors.Wrapf(err, "dropping kolet binary") + in, err := os.Open(kolet) + if err != nil { + return err + } + defer in.Close() + for _, m := range machines { + if _, err := in.Seek(0, 0); err != nil { + return errors.Wrapf(err, "seeking kolet binary") + } + if err := platform.InstallFile(in, m, remotepathpartial); err != nil { + return errors.Wrapf(err, "dropping kolet binary") + } + if out, stderr, err := m.SSH(fmt.Sprintf("sudo mv %s %s", remotepathpartial, remotepath)); err != nil { + return errors.Wrapf(err, "running sudo mv %s %s: %s: %s", remotepathpartial, remotepath, out, stderr) + } } return nil } diff --git a/mantle/kola/tests/ignition/resource.go b/mantle/kola/tests/ignition/resource.go index 903cd43760..f3624fec68 100644 --- a/mantle/kola/tests/ignition/resource.go +++ b/mantle/kola/tests/ignition/resource.go @@ -81,7 +81,7 @@ func init() { func resourceLocal(c cluster.TestCluster) { server := c.Machines()[0] - c.RunCmdSyncf(server, "sudo systemd-run --quiet ./kolet run %s Serve", c.H.Name()) + c.RunCmdSyncf(server, "sudo systemd-run --quiet /usr/local/bin/kolet run %s Serve", c.H.Name()) ip := server.PrivateIP() if c.Platform() == packet.Platform { diff --git a/mantle/kola/tests/ignition/security.go b/mantle/kola/tests/ignition/security.go index 50a8abaf5d..be18635686 100644 --- a/mantle/kola/tests/ignition/security.go +++ b/mantle/kola/tests/ignition/security.go @@ -95,7 +95,7 @@ EOF publicKey := c.MustSSH(server, "sudo cat /var/tls/server.crt") var conf *conf.UserData = localSecurityClient - c.RunCmdSyncf(server, "sudo systemd-run --quiet ./kolet run %s TLSServe", c.H.Name()) + c.RunCmdSyncf(server, "sudo systemd-run --quiet /usr/local/bin/kolet run %s TLSServe", c.H.Name()) client, err := c.NewMachine(conf.Subst("$IP", ip).Subst("$KEY", dataurl.EncodeBytes(publicKey))) if err != nil { diff --git a/mantle/kola/tests/upgrade/basic.go b/mantle/kola/tests/upgrade/basic.go index ab8fd37153..42217c2e97 100644 --- a/mantle/kola/tests/upgrade/basic.go +++ b/mantle/kola/tests/upgrade/basic.go @@ -80,11 +80,11 @@ func init() { { "name": "kolet-httpd.path", "enabled": true, - "contents": "[Path]\nPathExists=/var/home/core/kolet\n[Install]\nWantedBy=multi-user.target" + "contents": "[Path]\nPathExists=/usr/local/bin/kolet\n[Install]\nWantedBy=multi-user.target" }, { "name": "kolet-httpd.service", - "contents": "[Service]\nExecStart=/var/home/core/kolet run fcos.upgrade.basic httpd -v\n[Install]\nWantedBy=multi-user.target" + "contents": "[Service]\nExecStart=/usr/local/bin/kolet run fcos.upgrade.basic httpd -v\n[Install]\nWantedBy=multi-user.target" } ] },