Fix podman generate systemd --new special char handling

In a systemd unit dollar and percent signs are used for variables. A backslash is used for escape sequences. If any of these characters are used in the create command we have to properly escape them so systemd does not try to interpret them. Fixes containers#9176 Signed-off-by: Paul Holzinger <[email protected]>
mheon · Feb 4, 2021 · 3b712d9 · 3b712d9
1 parent be6afd1
commit 3b712d9
Show file tree

Hide file tree

Showing 5 changed files with 93 additions and 9 deletions.
diff --git a/pkg/systemd/generate/common.go b/pkg/systemd/generate/common.go
@@ -60,13 +60,21 @@ func filterPodFlags(command []string) []string {
 	return processed
 }
 
-// quoteArguments makes sure that all arguments with at least one whitespace
+// escapeSystemdArguments makes sure that all arguments with at least one whitespace
 // are quoted to make sure those are interpreted as one argument instead of
-// multiple ones.
-func quoteArguments(command []string) []string {
+// multiple ones. Also make sure to escape all characters which have a special
+// meaning to systemd -> $,% and \
+// see: https://www.freedesktop.org/software/systemd/man/systemd.service.html#Command%20lines
+func escapeSystemdArguments(command []string) []string {
 	for i := range command {
+		command[i] = strings.ReplaceAll(command[i], "$", "$$")
+		command[i] = strings.ReplaceAll(command[i], "%", "%%")
 		if strings.ContainsAny(command[i], " \t") {
 			command[i] = strconv.Quote(command[i])
+		} else if strings.Contains(command[i], `\`) {
+			// strconv.Quote also escapes backslashes so
+			// we should replace only if strconv.Quote was not used
+			command[i] = strings.ReplaceAll(command[i], `\`, `\\`)
 		}
 	}
 	return command

diff --git a/pkg/systemd/generate/common_test.go b/pkg/systemd/generate/common_test.go
@@ -29,7 +29,7 @@ func TestFilterPodFlags(t *testing.T) {
 	}
 }
 
-func TestQuoteArguments(t *testing.T) {
+func TestEscapeSystemdArguments(t *testing.T) {
 	tests := []struct {
 		input  []string
 		output []string
@@ -46,10 +46,46 @@ func TestQuoteArguments(t *testing.T) {
 			[]string{"foo", "bar=\"arg with\ttab\""},
 			[]string{"foo", "\"bar=\\\"arg with\\ttab\\\"\""},
 		},
+		{
+			[]string{"$"},
+			[]string{"$$"},
+		},
+		{
+			[]string{"foo", "command with dollar sign $"},
+			[]string{"foo", "\"command with dollar sign $$\""},
+		},
+		{
+			[]string{"foo", "command with two dollar signs $$"},
+			[]string{"foo", "\"command with two dollar signs $$$$\""},
+		},
+		{
+			[]string{"%"},
+			[]string{"%%"},
+		},
+		{
+			[]string{"foo", "command with percent sign %"},
+			[]string{"foo", "\"command with percent sign %%\""},
+		},
+		{
+			[]string{"foo", "command with two percent signs %%"},
+			[]string{"foo", "\"command with two percent signs %%%%\""},
+		},
+		{
+			[]string{`\`},
+			[]string{`\\`},
+		},
+		{
+			[]string{"foo", `command with backslash \`},
+			[]string{"foo", `"command with backslash \\"`},
+		},
+		{
+			[]string{"foo", `command with two backslashs \\`},
+			[]string{"foo", `"command with two backslashs \\\\"`},
+		},
 	}
 
 	for _, test := range tests {
-		quoted := quoteArguments(test.input)
+		quoted := escapeSystemdArguments(test.input)
 		assert.Equal(t, test.output, quoted)
 	}
 }
diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go
@@ -204,7 +204,7 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
 		startCommand := []string{info.Executable}
 		if index > 2 {
 			// include root flags
-			info.RootFlags = strings.Join(quoteArguments(info.CreateCommand[1:index-1]), " ")
+			info.RootFlags = strings.Join(escapeSystemdArguments(info.CreateCommand[1:index-1]), " ")
 			startCommand = append(startCommand, info.CreateCommand[1:index-1]...)
 		}
 		startCommand = append(startCommand,
@@ -279,7 +279,7 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
 			}
 		}
 		startCommand = append(startCommand, remainingCmd...)
-		startCommand = quoteArguments(startCommand)
+		startCommand = escapeSystemdArguments(startCommand)
 
 		info.ExecStartPre = "/bin/rm -f {{{{.PIDFile}}}} {{{{.ContainerIDFile}}}}"
 		info.ExecStart = strings.Join(startCommand, " ")

diff --git a/pkg/systemd/generate/containers_test.go b/pkg/systemd/generate/containers_test.go
@@ -349,6 +349,30 @@ ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id
 PIDFile=%t/jadda-jadda.pid
 Type=forking
 
+[Install]
+WantedBy=multi-user.target default.target
+`
+
+	goodNewWithSpecialChars := `# jadda-jadda.service
+# autogenerated by Podman CI
+
+[Unit]
+Description=Podman jadda-jadda.service
+Documentation=man:podman-generate-systemd(1)
+Wants=network.target
+After=network-online.target
+
+[Service]
+Environment=PODMAN_SYSTEMD_UNIT=%n
+Restart=always
+TimeoutStopSec=70
+ExecStartPre=/bin/rm -f %t/jadda-jadda.pid %t/jadda-jadda.ctr-id
+ExecStart=/usr/bin/podman run --conmon-pidfile %t/jadda-jadda.pid --cidfile %t/jadda-jadda.ctr-id --cgroups=no-conmon -d --replace --name test awesome-image:latest sh -c "kill $$$$ && echo %%\\"
+ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 10
+ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id
+PIDFile=%t/jadda-jadda.pid
+Type=forking
+
 [Install]
 WantedBy=multi-user.target default.target
 `
@@ -647,6 +671,22 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 		},
+		{"good with special chars",
+			containerInfo{
+				Executable:        "/usr/bin/podman",
+				ServiceName:       "jadda-jadda",
+				ContainerNameOrID: "jadda-jadda",
+				RestartPolicy:     "always",
+				PIDFile:           "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
+				StopTimeout:       10,
+				PodmanVersion:     "CI",
+				CreateCommand:     []string{"I'll get stripped", "create", "--name", "test", "awesome-image:latest", "sh", "-c", "kill $$ && echo %\\"},
+				EnvVariable:       EnvVariable,
+			},
+			goodNewWithSpecialChars,
+			true,
+			false,
+		},
 	}
 	for _, tt := range tests {
 		test := tt

diff --git a/pkg/systemd/generate/pods.go b/pkg/systemd/generate/pods.go
@@ -269,7 +269,7 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
 				return "", errors.Errorf("pod does not appear to be created via `podman pod create`: %v", info.CreateCommand)
 			}
 			podRootArgs = info.CreateCommand[1 : podCreateIndex-1]
-			info.RootFlags = strings.Join(quoteArguments(podRootArgs), " ")
+			info.RootFlags = strings.Join(escapeSystemdArguments(podRootArgs), " ")
 			podCreateArgs = filterPodFlags(info.CreateCommand[podCreateIndex+1:])
 		}
 		// We're hard-coding the first five arguments and append the
@@ -306,7 +306,7 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
 		}
 
 		startCommand = append(startCommand, podCreateArgs...)
-		startCommand = quoteArguments(startCommand)
+		startCommand = escapeSystemdArguments(startCommand)
 
 		info.ExecStartPre1 = "/bin/rm -f {{{{.PIDFile}}}} {{{{.PodIDFile}}}}"
 		info.ExecStartPre2 = strings.Join(startCommand, " ")