From d19ea6a60de8fbe11e38c5a9fefbdbc799d1dd40 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Fri, 2 Dec 2022 16:22:29 +0100 Subject: [PATCH 1/5] quadlet: Change NoNewPrivileges default to false This matches the default of podman run. Signed-off-by: Alexander Larsson --- docs/source/markdown/podman-systemd.unit.5.md | 2 +- pkg/systemd/quadlet/quadlet.go | 2 +- test/e2e/quadlet/basic.container | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/source/markdown/podman-systemd.unit.5.md b/docs/source/markdown/podman-systemd.unit.5.md index 4f2b1a518c..3bad70bdc5 100644 --- a/docs/source/markdown/podman-systemd.unit.5.md +++ b/docs/source/markdown/podman-systemd.unit.5.md @@ -109,7 +109,7 @@ which can be modified with `RemapUsers`, but if that is not specified, this uid The (numeric) gid to run as inside the container. This does not need to match the gid on the host, which can be modified with `RemapUsers`, but if that is not specified, this gid is also used on the host. -#### `NoNewPrivileges=` (defaults to `yes`) +#### `NoNewPrivileges=` (defaults to `no`) If enabled (which is the default), this disables the container processes from gaining additional privileges via things like setuid and file capabilities. diff --git a/pkg/systemd/quadlet/quadlet.go b/pkg/systemd/quadlet/quadlet.go index 32f11db73c..840cb8c4a0 100644 --- a/pkg/systemd/quadlet/quadlet.go +++ b/pkg/systemd/quadlet/quadlet.go @@ -297,7 +297,7 @@ func ConvertContainer(container *parser.UnitFile, isUser bool) (*parser.UnitFile } // Default to no higher level privileges or caps - noNewPrivileges := container.LookupBoolean(ContainerGroup, KeyNoNewPrivileges, true) + noNewPrivileges := container.LookupBoolean(ContainerGroup, KeyNoNewPrivileges, false) if noNewPrivileges { podman.add("--security-opt=no-new-privileges") } diff --git a/test/e2e/quadlet/basic.container b/test/e2e/quadlet/basic.container index 8369f75aa9..9929e6ec41 100644 --- a/test/e2e/quadlet/basic.container +++ b/test/e2e/quadlet/basic.container @@ -10,7 +10,6 @@ ## assert-podman-args "--runtime" "/usr/bin/crun" ## assert-podman-args "--cgroups=split" ## assert-podman-args "--sdnotify=conmon" -## assert-podman-args "--security-opt=no-new-privileges" ## assert-podman-args "--cap-drop=all" ## assert-podman-args "--read-only" ## !assert-podman-args "--read-only-tmpfs=false" From 1c3fddfaf72f24f94da4d3d1a473ab2abeaa77b7 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Fri, 2 Dec 2022 16:25:47 +0100 Subject: [PATCH 2/5] quadlet: Change RunInit default to no This matches the default of podman run. Signed-off-by: Alexander Larsson --- docs/source/markdown/podman-systemd.unit.5.md | 4 ++-- pkg/systemd/quadlet/quadlet.go | 2 +- test/e2e/quadlet/basic.container | 1 - 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/source/markdown/podman-systemd.unit.5.md b/docs/source/markdown/podman-systemd.unit.5.md index 3bad70bdc5..7f62cb67c6 100644 --- a/docs/source/markdown/podman-systemd.unit.5.md +++ b/docs/source/markdown/podman-systemd.unit.5.md @@ -190,9 +190,9 @@ of startup on its own. The timezone to run the container in. -#### `RunInit=` (default to `yes`) +#### `RunInit=` (default to `no`) -If enabled (and it is by default), the container will have a minimal init process inside the +If enabled, the container will have a minimal init process inside the container that forwards signals and reaps processes. #### `VolatileTmp=` (default to `yes`) diff --git a/pkg/systemd/quadlet/quadlet.go b/pkg/systemd/quadlet/quadlet.go index 840cb8c4a0..45560680ec 100644 --- a/pkg/systemd/quadlet/quadlet.go +++ b/pkg/systemd/quadlet/quadlet.go @@ -276,7 +276,7 @@ func ConvertContainer(container *parser.UnitFile, isUser bool) (*parser.UnitFile } // Run with a pid1 init to reap zombies by default (as most apps don't do that) - runInit := container.LookupBoolean(ContainerGroup, KeyRunInit, true) + runInit := container.LookupBoolean(ContainerGroup, KeyRunInit, false) if runInit { podman.add("--init") } diff --git a/test/e2e/quadlet/basic.container b/test/e2e/quadlet/basic.container index 9929e6ec41..21e04efdda 100644 --- a/test/e2e/quadlet/basic.container +++ b/test/e2e/quadlet/basic.container @@ -6,7 +6,6 @@ ## assert-podman-args "-d" ## assert-podman-args "--log-driver" "passthrough" ## assert-podman-args "--pull=never" -## assert-podman-args "--init" ## assert-podman-args "--runtime" "/usr/bin/crun" ## assert-podman-args "--cgroups=split" ## assert-podman-args "--sdnotify=conmon" From 098ad52ecb09a7e24eb47c83aee8f734774b2070 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Fri, 2 Dec 2022 16:30:15 +0100 Subject: [PATCH 3/5] quadlet: Change default of ReadOnly to no This matches the default podman run behaviour. Signed-off-by: Alexander Larsson --- docs/source/markdown/podman-systemd.unit.5.md | 2 +- pkg/systemd/quadlet/quadlet.go | 2 +- test/e2e/quadlet/basic.container | 2 -- test/e2e/quadlet/readonly-notmpfs.container | 1 + 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/source/markdown/podman-systemd.unit.5.md b/docs/source/markdown/podman-systemd.unit.5.md index 7f62cb67c6..1401c708ff 100644 --- a/docs/source/markdown/podman-systemd.unit.5.md +++ b/docs/source/markdown/podman-systemd.unit.5.md @@ -138,7 +138,7 @@ For example: AddCapability=CAP_DAC_OVERRIDE CAP_IPC_OWNER ``` -#### `ReadOnly=` (defaults to `yes`) +#### `ReadOnly=` (defaults to `no`) If enabled, makes image read-only, with /var/tmp, /tmp and /run a tmpfs (unless disabled by `VolatileTmp=no`). diff --git a/pkg/systemd/quadlet/quadlet.go b/pkg/systemd/quadlet/quadlet.go index 45560680ec..8ba00a88dd 100644 --- a/pkg/systemd/quadlet/quadlet.go +++ b/pkg/systemd/quadlet/quadlet.go @@ -329,7 +329,7 @@ func ConvertContainer(container *parser.UnitFile, isUser bool) (*parser.UnitFile podman.addf("--cap-add=%s", strings.ToLower(caps)) } - readOnly := container.LookupBoolean(ContainerGroup, KeyReadOnly, true) + readOnly := container.LookupBoolean(ContainerGroup, KeyReadOnly, false) if readOnly { podman.add("--read-only") } diff --git a/test/e2e/quadlet/basic.container b/test/e2e/quadlet/basic.container index 21e04efdda..78372f27bc 100644 --- a/test/e2e/quadlet/basic.container +++ b/test/e2e/quadlet/basic.container @@ -10,8 +10,6 @@ ## assert-podman-args "--cgroups=split" ## assert-podman-args "--sdnotify=conmon" ## assert-podman-args "--cap-drop=all" -## assert-podman-args "--read-only" -## !assert-podman-args "--read-only-tmpfs=false" ## assert-key-is "Unit" "RequiresMountsFor" "%t/containers" ## assert-key-is "Service" "KillMode" "mixed" ## assert-key-is "Service" "Delegate" "yes" diff --git a/test/e2e/quadlet/readonly-notmpfs.container b/test/e2e/quadlet/readonly-notmpfs.container index cddc7b7142..88087cec39 100644 --- a/test/e2e/quadlet/readonly-notmpfs.container +++ b/test/e2e/quadlet/readonly-notmpfs.container @@ -3,4 +3,5 @@ [Container] Image=localhost/imagename +ReadOnly=yes VolatileTmp=no From b34ab8b5fa2607ff28582243a75b2e9c14a7403f Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Fri, 2 Dec 2022 16:33:09 +0100 Subject: [PATCH 4/5] quadlet: Drop --pull=never This is to match podman run default behaviour. Signed-off-by: Alexander Larsson --- pkg/systemd/quadlet/quadlet.go | 4 +--- test/e2e/quadlet/basepodman.container | 2 +- test/e2e/quadlet/basic.container | 1 - 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/pkg/systemd/quadlet/quadlet.go b/pkg/systemd/quadlet/quadlet.go index 8ba00a88dd..2268c755b4 100644 --- a/pkg/systemd/quadlet/quadlet.go +++ b/pkg/systemd/quadlet/quadlet.go @@ -253,9 +253,7 @@ func ConvertContainer(container *parser.UnitFile, isUser bool) (*parser.UnitFile // But we still want output to the journal, so use the log driver. "--log-driver", "passthrough", - - // Never try to pull the image during service start - "--pull=never") + ) // We use crun as the runtime and delegated groups to it service.Add(ServiceGroup, "Delegate", "yes") diff --git a/test/e2e/quadlet/basepodman.container b/test/e2e/quadlet/basepodman.container index 5ac5b962a8..6c6de8abb5 100644 --- a/test/e2e/quadlet/basepodman.container +++ b/test/e2e/quadlet/basepodman.container @@ -1,4 +1,4 @@ -## assert-podman-final-args run --name=systemd-%N --cidfile=%t/%N.cid --replace --rm -d --log-driver passthrough --pull=never --runtime /usr/bin/crun --cgroups=split --sdnotify=conmon localhost/imagename +## assert-podman-final-args run --name=systemd-%N --cidfile=%t/%N.cid --replace --rm -d --log-driver passthrough --runtime /usr/bin/crun --cgroups=split --sdnotify=conmon localhost/imagename [Container] Image=localhost/imagename diff --git a/test/e2e/quadlet/basic.container b/test/e2e/quadlet/basic.container index 78372f27bc..4d248c2a6f 100644 --- a/test/e2e/quadlet/basic.container +++ b/test/e2e/quadlet/basic.container @@ -5,7 +5,6 @@ ## assert-podman-args "--replace" ## assert-podman-args "-d" ## assert-podman-args "--log-driver" "passthrough" -## assert-podman-args "--pull=never" ## assert-podman-args "--runtime" "/usr/bin/crun" ## assert-podman-args "--cgroups=split" ## assert-podman-args "--sdnotify=conmon" From 16cf34dc3a6b9ee1d4eb7d73b58437282f86946b Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Fri, 2 Dec 2022 16:37:22 +0100 Subject: [PATCH 5/5] quadlet: Use same default capability set as podman run Signed-off-by: Alexander Larsson --- docs/source/markdown/podman-systemd.unit.5.md | 5 ++--- pkg/systemd/quadlet/quadlet.go | 5 +---- test/e2e/quadlet/basic.container | 1 - test/e2e/quadlet/capabilities.container | 2 -- 4 files changed, 3 insertions(+), 10 deletions(-) diff --git a/docs/source/markdown/podman-systemd.unit.5.md b/docs/source/markdown/podman-systemd.unit.5.md index 1401c708ff..366a9989d3 100644 --- a/docs/source/markdown/podman-systemd.unit.5.md +++ b/docs/source/markdown/podman-systemd.unit.5.md @@ -116,8 +116,7 @@ setuid and file capabilities. #### `DropCapability=` (defaults to `all`) -Drop these capabilities from the default podman capability set, or `all` for all capabilities. The default if no -`DropCapability` is set is `all`. Set this to empty (i.e. `DropCapability=`) to use the default podman capability set. +Drop these capabilities from the default podman capability set, or `all` to drop all capabilities. This is a space separated list of capabilities. This key can be listed multiple times. @@ -140,7 +139,7 @@ AddCapability=CAP_DAC_OVERRIDE CAP_IPC_OWNER #### `ReadOnly=` (defaults to `no`) -If enabled, makes image read-only, with /var/tmp, /tmp and /run a tmpfs (unless disabled by `VolatileTmp=no`). +If enabled, makes image read-only, with /var/tmp, /tmp and /run a tmpfs (unless disabled by `VolatileTmp=no`).r **NOTE:** Podman will automatically copy any content from the image onto the tmpfs diff --git a/pkg/systemd/quadlet/quadlet.go b/pkg/systemd/quadlet/quadlet.go index 2268c755b4..56cb1655fb 100644 --- a/pkg/systemd/quadlet/quadlet.go +++ b/pkg/systemd/quadlet/quadlet.go @@ -312,10 +312,7 @@ func ConvertContainer(container *parser.UnitFile, isUser bool) (*parser.UnitFile podman.add("--security-opt", fmt.Sprintf("seccomp=%s", seccompProfile)) } - dropCaps := []string{"all"} // Default - if container.HasKey(ContainerGroup, KeyDropCapability) { - dropCaps = container.LookupAllStrv(ContainerGroup, KeyDropCapability) - } + dropCaps := container.LookupAllStrv(ContainerGroup, KeyDropCapability) for _, caps := range dropCaps { podman.addf("--cap-drop=%s", strings.ToLower(caps)) diff --git a/test/e2e/quadlet/basic.container b/test/e2e/quadlet/basic.container index 4d248c2a6f..6eef693c22 100644 --- a/test/e2e/quadlet/basic.container +++ b/test/e2e/quadlet/basic.container @@ -8,7 +8,6 @@ ## assert-podman-args "--runtime" "/usr/bin/crun" ## assert-podman-args "--cgroups=split" ## assert-podman-args "--sdnotify=conmon" -## assert-podman-args "--cap-drop=all" ## assert-key-is "Unit" "RequiresMountsFor" "%t/containers" ## assert-key-is "Service" "KillMode" "mixed" ## assert-key-is "Service" "Delegate" "yes" diff --git a/test/e2e/quadlet/capabilities.container b/test/e2e/quadlet/capabilities.container index d99e30e265..74c21a869e 100644 --- a/test/e2e/quadlet/capabilities.container +++ b/test/e2e/quadlet/capabilities.container @@ -5,7 +5,5 @@ [Container] Image=localhost/imagename -# Verify that we can reset to the default cap set -DropCapability= AddCapability=CAP_DAC_OVERRIDE CAP_AUDIT_WRITE AddCapability=CAP_IPC_OWNER