diff --git a/cli/connhelper/connhelper.go b/cli/connhelper/connhelper.go index f4b9388599..25ce7aef02 100644 --- a/cli/connhelper/connhelper.go +++ b/cli/connhelper/connhelper.go @@ -47,14 +47,19 @@ func getConnectionHelper(daemonURL string, sshFlags []string) (*ConnectionHelper } sshFlags = addSSHTimeout(sshFlags) sshFlags = disablePseudoTerminalAllocation(sshFlags) + + remoteCommand := []string{"docker", "system", "dial-stdio"} + socketPath := sp.Path + if strings.Trim(sp.Path, "/") != "" { + remoteCommand = []string{"docker", "--host=unix://" + socketPath, "system", "dial-stdio"} + } + sshArgs, err := sp.Command(sshFlags, remoteCommand...) + if err != nil { + return nil, err + } return &ConnectionHelper{ Dialer: func(ctx context.Context, network, addr string) (net.Conn, error) { - args := []string{"docker"} - if sp.Path != "" { - args = append(args, "--host", "unix://"+sp.Path) - } - args = append(args, "system", "dial-stdio") - return commandconn.New(ctx, "ssh", append(sshFlags, sp.Args(args...)...)...) + return commandconn.New(ctx, "ssh", sshArgs...) }, Host: "http://docker.example.com", }, nil diff --git a/cli/connhelper/internal/syntax/LICENSE b/cli/connhelper/internal/syntax/LICENSE new file mode 100644 index 0000000000..2a5268e5f1 --- /dev/null +++ b/cli/connhelper/internal/syntax/LICENSE @@ -0,0 +1,27 @@ +Copyright (c) 2016, Daniel Martí. All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + + * Redistributions of source code must retain the above copyright +notice, this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above +copyright notice, this list of conditions and the following disclaimer +in the documentation and/or other materials provided with the +distribution. + * Neither the name of the copyright holder nor the names of its +contributors may be used to endorse or promote products derived from +this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/cli/connhelper/internal/syntax/doc.go b/cli/connhelper/internal/syntax/doc.go new file mode 100644 index 0000000000..32cf60c7c6 --- /dev/null +++ b/cli/connhelper/internal/syntax/doc.go @@ -0,0 +1,13 @@ +// Package syntax is a fork of [mvdan.cc/sh/v3@v3.10.0/syntax]. +// +// Copyright (c) 2016, Daniel Martí. All rights reserved. +// +// It is a reduced set of the package to only provide the [Quote] function, +// and contains the [LICENSE], [quote.go] and [parser.go] files at the given +// revision. +// +// [quote.go]: https://raw.githubusercontent.com/mvdan/sh/refs/tags/v3.10.0/syntax/quote.go +// [parser.go]: https://raw.githubusercontent.com/mvdan/sh/refs/tags/v3.10.0/syntax/parser.go +// [LICENSE]: https://raw.githubusercontent.com/mvdan/sh/refs/tags/v3.10.0/LICENSE +// [mvdan.cc/sh/v3@v3.10.0/syntax]: https://pkg.go.dev/mvdan.cc/sh/v3@v3.10.0/syntax +package syntax diff --git a/cli/connhelper/internal/syntax/parser.go b/cli/connhelper/internal/syntax/parser.go new file mode 100644 index 0000000000..06b1222f43 --- /dev/null +++ b/cli/connhelper/internal/syntax/parser.go @@ -0,0 +1,95 @@ +// Copyright (c) 2016, Daniel Martí +// See LICENSE for licensing information + +package syntax + +// LangVariant describes a shell language variant to use when tokenizing and +// parsing shell code. The zero value is [LangBash]. +type LangVariant int + +const ( + // LangBash corresponds to the GNU Bash language, as described in its + // manual at https://www.gnu.org/software/bash/manual/bash.html. + // + // We currently follow Bash version 5.2. + // + // Its string representation is "bash". + LangBash LangVariant = iota + + // LangPOSIX corresponds to the POSIX Shell language, as described at + // https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html. + // + // Its string representation is "posix" or "sh". + LangPOSIX + + // LangMirBSDKorn corresponds to the MirBSD Korn Shell, also known as + // mksh, as described at http://www.mirbsd.org/htman/i386/man1/mksh.htm. + // Note that it shares some features with Bash, due to the shared + // ancestry that is ksh. + // + // We currently follow mksh version 59. + // + // Its string representation is "mksh". + LangMirBSDKorn + + // LangBats corresponds to the Bash Automated Testing System language, + // as described at https://github.com/bats-core/bats-core. Note that + // it's just a small extension of the Bash language. + // + // Its string representation is "bats". + LangBats + + // LangAuto corresponds to automatic language detection, + // commonly used by end-user applications like shfmt, + // which can guess a file's language variant given its filename or shebang. + // + // At this time, [Variant] does not support LangAuto. + LangAuto +) + +func (l LangVariant) String() string { + switch l { + case LangBash: + return "bash" + case LangPOSIX: + return "posix" + case LangMirBSDKorn: + return "mksh" + case LangBats: + return "bats" + case LangAuto: + return "auto" + } + return "unknown shell language variant" +} + +// IsKeyword returns true if the given word is part of the language keywords. +func IsKeyword(word string) bool { + // This list has been copied from the bash 5.1 source code, file y.tab.c +4460 + switch word { + case + "!", + "[[", // only if COND_COMMAND is defined + "]]", // only if COND_COMMAND is defined + "case", + "coproc", // only if COPROCESS_SUPPORT is defined + "do", + "done", + "else", + "esac", + "fi", + "for", + "function", + "if", + "in", + "select", // only if SELECT_COMMAND is defined + "then", + "time", // only if COMMAND_TIMING is defined + "until", + "while", + "{", + "}": + return true + } + return false +} diff --git a/cli/connhelper/internal/syntax/quote.go b/cli/connhelper/internal/syntax/quote.go new file mode 100644 index 0000000000..628fa4891c --- /dev/null +++ b/cli/connhelper/internal/syntax/quote.go @@ -0,0 +1,187 @@ +// Copyright (c) 2021, Daniel Martí +// See LICENSE for licensing information + +package syntax + +import ( + "fmt" + "strings" + "unicode" + "unicode/utf8" +) + +type QuoteError struct { + ByteOffset int + Message string +} + +func (e QuoteError) Error() string { + return fmt.Sprintf("cannot quote character at byte %d: %s", e.ByteOffset, e.Message) +} + +const ( + quoteErrNull = "shell strings cannot contain null bytes" + quoteErrPOSIX = "POSIX shell lacks escape sequences" + quoteErrRange = "rune out of range" + quoteErrMksh = "mksh cannot escape codepoints above 16 bits" +) + +// Quote returns a quoted version of the input string, +// so that the quoted version is expanded or interpreted +// as the original string in the given language variant. +// +// Quoting is necessary when using arbitrary literal strings +// as words in a shell script or command. +// Without quoting, one can run into syntax errors, +// as well as the possibility of running unintended code. +// +// An error is returned when a string cannot be quoted for a variant. +// For instance, POSIX lacks escape sequences for non-printable characters, +// and no language variant can represent a string containing null bytes. +// In such cases, the returned error type will be *QuoteError. +// +// The quoting strategy is chosen on a best-effort basis, +// to minimize the amount of extra bytes necessary. +// +// Some strings do not require any quoting and are returned unchanged. +// Those strings can be directly surrounded in single quotes as well. +// +//nolint:gocyclo // ignore "cyclomatic complexity 35 of func `Quote` is high (> 16) (gocyclo)" +func Quote(s string, lang LangVariant) (string, error) { + if s == "" { + // Special case; an empty string must always be quoted, + // as otherwise it expands to zero fields. + return "''", nil + } + shellChars := false + nonPrintable := false + offs := 0 + for rem := s; len(rem) > 0; { + r, size := utf8.DecodeRuneInString(rem) + switch r { + // Like regOps; token characters. + case ';', '"', '\'', '(', ')', '$', '|', '&', '>', '<', '`', + // Whitespace; might result in multiple fields. + ' ', '\t', '\r', '\n', + // Escape sequences would be expanded. + '\\', + // Would start a comment unless quoted. + '#', + // Might result in brace expansion. + '{', + // Might result in tilde expansion. + '~', + // Might result in globbing. + '*', '?', '[', + // Might result in an assignment. + '=': + shellChars = true + case '\x00': + return "", &QuoteError{ByteOffset: offs, Message: quoteErrNull} + } + if r == utf8.RuneError || !unicode.IsPrint(r) { + if lang == LangPOSIX { + return "", &QuoteError{ByteOffset: offs, Message: quoteErrPOSIX} + } + nonPrintable = true + } + rem = rem[size:] + offs += size + } + if !shellChars && !nonPrintable && !IsKeyword(s) { + // Nothing to quote; avoid allocating. + return s, nil + } + + // Single quotes are usually best, + // as they don't require any escaping of characters. + // If we have any invalid utf8 or non-printable runes, + // use $'' so that we can escape them. + // Note that we can't use double quotes for those. + var b strings.Builder + if nonPrintable { + b.WriteString("$'") + lastRequoteIfHex := false + offs = 0 + for rem := s; len(rem) > 0; { + nextRequoteIfHex := false + r, size := utf8.DecodeRuneInString(rem) + switch { + case r == '\'', r == '\\': + b.WriteByte('\\') + b.WriteRune(r) + case unicode.IsPrint(r) && r != utf8.RuneError: + if lastRequoteIfHex && isHex(r) { + b.WriteString("'$'") + } + b.WriteRune(r) + case r == '\a': + b.WriteString(`\a`) + case r == '\b': + b.WriteString(`\b`) + case r == '\f': + b.WriteString(`\f`) + case r == '\n': + b.WriteString(`\n`) + case r == '\r': + b.WriteString(`\r`) + case r == '\t': + b.WriteString(`\t`) + case r == '\v': + b.WriteString(`\v`) + case r < utf8.RuneSelf, r == utf8.RuneError && size == 1: + // \xXX, fixed at two hexadecimal characters. + fmt.Fprintf(&b, "\\x%02x", rem[0]) + // Unfortunately, mksh allows \x to consume more hex characters. + // Ensure that we don't allow it to read more than two. + if lang == LangMirBSDKorn { + nextRequoteIfHex = true + } + case r > utf8.MaxRune: + // Not a valid Unicode code point? + return "", &QuoteError{ByteOffset: offs, Message: quoteErrRange} + case lang == LangMirBSDKorn && r > 0xFFFD: + // From the CAVEATS section in R59's man page: + // + // mksh currently uses OPTU-16 internally, which is the same as + // UTF-8 and CESU-8 with 0000..FFFD being valid codepoints. + return "", &QuoteError{ByteOffset: offs, Message: quoteErrMksh} + case r < 0x10000: + // \uXXXX, fixed at four hexadecimal characters. + fmt.Fprintf(&b, "\\u%04x", r) + default: + // \UXXXXXXXX, fixed at eight hexadecimal characters. + fmt.Fprintf(&b, "\\U%08x", r) + } + rem = rem[size:] + lastRequoteIfHex = nextRequoteIfHex + offs += size + } + b.WriteString("'") + return b.String(), nil + } + + // Single quotes without any need for escaping. + if !strings.Contains(s, "'") { + return "'" + s + "'", nil + } + + // The string contains single quotes, + // so fall back to double quotes. + b.WriteByte('"') + for _, r := range s { + switch r { + case '"', '\\', '`', '$': + b.WriteByte('\\') + } + b.WriteRune(r) + } + b.WriteByte('"') + return b.String(), nil +} + +func isHex(r rune) bool { + return (r >= '0' && r <= '9') || + (r >= 'a' && r <= 'f') || + (r >= 'A' && r <= 'F') +} diff --git a/cli/connhelper/ssh/ssh.go b/cli/connhelper/ssh/ssh.go index 2c9d0d61b1..2fcb54a98f 100644 --- a/cli/connhelper/ssh/ssh.go +++ b/cli/connhelper/ssh/ssh.go @@ -5,6 +5,8 @@ import ( "errors" "fmt" "net/url" + + "github.com/docker/cli/cli/connhelper/internal/syntax" ) // ParseURL creates a [Spec] from the given ssh URL. It returns an error if @@ -76,16 +78,106 @@ type Spec struct { Path string } -// Args returns args except "ssh" itself combined with optional additional command args -func (sp *Spec) Args(add ...string) []string { +// Args returns args except "ssh" itself combined with optional additional +// command and args to be executed on the remote host. It attempts to quote +// the given arguments to account for ssh executing the remote command in a +// shell. It returns nil when unable to quote the remote command. +func (sp *Spec) Args(remoteCommandAndArgs ...string) []string { + // Format the remote command to run using the ssh connection, quoting + // values where needed because ssh executes these in a POSIX shell. + remoteCommand, err := quoteCommand(remoteCommandAndArgs...) + if err != nil { + return nil + } + + sshArgs, err := sp.args() + if err != nil { + return nil + } + if remoteCommand != "" { + sshArgs = append(sshArgs, remoteCommand) + } + return sshArgs +} + +func (sp *Spec) args(sshFlags ...string) ([]string, error) { var args []string + if sp.Host == "" { + return nil, errors.New("no host specified") + } if sp.User != "" { - args = append(args, "-l", sp.User) + // Quote user, as it's obtained from the URL. + usr, err := syntax.Quote(sp.User, syntax.LangPOSIX) + if err != nil { + return nil, fmt.Errorf("invalid user: %w", err) + } + args = append(args, "-l", usr) } if sp.Port != "" { - args = append(args, "-p", sp.Port) + // Quote port, as it's obtained from the URL. + port, err := syntax.Quote(sp.Port, syntax.LangPOSIX) + if err != nil { + return nil, fmt.Errorf("invalid port: %w", err) + } + args = append(args, "-p", port) } - args = append(args, "--", sp.Host) - args = append(args, add...) - return args + + // We consider "sshFlags" to be "trusted", and set from code only, + // as they are not parsed from the DOCKER_HOST URL. + args = append(args, sshFlags...) + + host, err := syntax.Quote(sp.Host, syntax.LangPOSIX) + if err != nil { + return nil, fmt.Errorf("invalid host: %w", err) + } + + return append(args, "--", host), nil +} + +// Command returns the ssh flags and arguments to execute a command +// (remoteCommandAndArgs) on the remote host. Where needed, it quotes +// values passed in remoteCommandAndArgs to account for ssh executing +// the remote command in a shell. It returns an error if no remote command +// is passed, or when unable to quote the remote command. +// +// Important: to preserve backward-compatibility, Command does not currently +// perform sanitization or quoting on the sshFlags and callers are expected +// to sanitize this argument. +func (sp *Spec) Command(sshFlags []string, remoteCommandAndArgs ...string) ([]string, error) { + if len(remoteCommandAndArgs) == 0 { + return nil, errors.New("no remote command specified") + } + sshArgs, err := sp.args(sshFlags...) + if err != nil { + return nil, err + } + remoteCommand, err := quoteCommand(remoteCommandAndArgs...) + if err != nil { + return nil, err + } + if remoteCommand != "" { + sshArgs = append(sshArgs, remoteCommand) + } + return sshArgs, nil +} + +// quoteCommand returns the remote command to run using the ssh connection +// as a single string, quoting values where needed because ssh executes +// these in a POSIX shell. +func quoteCommand(commandAndArgs ...string) (string, error) { + var quotedCmd string + for i, arg := range commandAndArgs { + a, err := syntax.Quote(arg, syntax.LangPOSIX) + if err != nil { + return "", fmt.Errorf("invalid argument: %w", err) + } + if i == 0 { + quotedCmd = a + continue + } + quotedCmd += " " + a + } + // each part is quoted appropriately, so now we'll have a full + // shell command to pass off to "ssh" + return quotedCmd, nil } diff --git a/cli/connhelper/ssh/ssh_test.go b/cli/connhelper/ssh/ssh_test.go index 492408f116..6de0da0d2d 100644 --- a/cli/connhelper/ssh/ssh_test.go +++ b/cli/connhelper/ssh/ssh_test.go @@ -1,6 +1,7 @@ package ssh import ( + "strings" "testing" "gotest.tools/v3/assert" @@ -26,6 +27,28 @@ func TestParseURL(t *testing.T) { Host: "example.com", }, }, + { + doc: "bare ssh URL with trailing slash", + url: "ssh://example.com/", + expectedArgs: []string{ + "--", "example.com", + }, + expectedSpec: Spec{ + Host: "example.com", + Path: "/", + }, + }, + { + doc: "bare ssh URL with trailing slashes", + url: "ssh://example.com//", + expectedArgs: []string{ + "--", "example.com", + }, + expectedSpec: Spec{ + Host: "example.com", + Path: "//", + }, + }, { doc: "bare ssh URL and remote command", url: "ssh://example.com", @@ -34,7 +57,7 @@ func TestParseURL(t *testing.T) { }, expectedArgs: []string{ "--", "example.com", - "docker", "system", "dial-stdio", + `docker system dial-stdio`, }, expectedSpec: Spec{ Host: "example.com", @@ -48,7 +71,7 @@ func TestParseURL(t *testing.T) { }, expectedArgs: []string{ "--", "example.com", - "docker", "--host", "unix:///var/run/docker.sock", "system", "dial-stdio", + `docker --host unix:///var/run/docker.sock system dial-stdio`, }, expectedSpec: Spec{ Host: "example.com", @@ -84,6 +107,25 @@ func TestParseURL(t *testing.T) { Path: "/var/run/docker.sock", }, }, + { + // This test is only to verify the behavior of ParseURL to + // pass through the Path as-is. Neither Spec.Args, nor + // Spec.Command use the Path field directly, and it should + // likely be deprecated. + doc: "bad path", + url: `ssh://example.com/var/run/docker.sock '$(echo hello > /hello.txt)'`, + remoteCommand: []string{ + "docker", "--host", `unix:///var/run/docker.sock '$(echo hello > /hello.txt)'`, "system", "dial-stdio", + }, + expectedArgs: []string{ + "--", "example.com", + `docker --host "unix:///var/run/docker.sock '\$(echo hello > /hello.txt)'" system dial-stdio`, + }, + expectedSpec: Spec{ + Host: "example.com", + Path: `/var/run/docker.sock '$(echo hello > /hello.txt)'`, + }, + }, { doc: "malformed URL", url: "malformed %%url", @@ -123,6 +165,21 @@ func TestParseURL(t *testing.T) { url: "https://example.com", expectedError: `invalid SSH URL: incorrect scheme: https`, }, + { + doc: "invalid URL with NUL character", + url: "ssh://example.com/var/run/\x00docker.sock", + expectedError: `invalid SSH URL: net/url: invalid control character in URL`, + }, + { + doc: "invalid URL with newline character", + url: "ssh://example.com/var/run/docker.sock\n", + expectedError: `invalid SSH URL: net/url: invalid control character in URL`, + }, + { + doc: "invalid URL with control character", + url: "ssh://example.com/var/run/\x1bdocker.sock", + expectedError: `invalid SSH URL: net/url: invalid control character in URL`, + }, } for _, tc := range testCases { t.Run(tc.doc, func(t *testing.T) { @@ -139,3 +196,122 @@ func TestParseURL(t *testing.T) { }) } } + +func TestCommand(t *testing.T) { + testCases := []struct { + doc string + url string + sshFlags []string + customCmd []string + expectedCmd []string + expectedError string + }{ + { + doc: "bare ssh URL", + url: "ssh://example.com", + expectedCmd: []string{ + "--", "example.com", + "docker system dial-stdio", + }, + }, + { + doc: "bare ssh URL with trailing slash", + url: "ssh://example.com/", + expectedCmd: []string{ + "--", "example.com", + "docker system dial-stdio", + }, + }, + { + doc: "bare ssh URL with custom ssh flags", + url: "ssh://example.com", + sshFlags: []string{"-T", "-o", "ConnectTimeout=30", "-oStrictHostKeyChecking=no"}, + expectedCmd: []string{ + "-T", + "-o", "ConnectTimeout=30", + "-oStrictHostKeyChecking=no", + "--", "example.com", + "docker system dial-stdio", + }, + }, + { + doc: "ssh URL with all options", + url: "ssh://me@example.com:10022/var/run/docker.sock", + sshFlags: []string{"-T", "-o ConnectTimeout=30"}, + expectedCmd: []string{ + "-l", "me", + "-p", "10022", + "-T", + "-o ConnectTimeout=30", + "--", "example.com", + "docker '--host=unix:///var/run/docker.sock' system dial-stdio", + }, + }, + { + doc: "bad ssh flags", + url: "ssh://example.com", + sshFlags: []string{"-T", "-o", `ConnectTimeout=30 $(echo hi > /hi.txt)`}, + expectedCmd: []string{ + "-T", + "-o", `ConnectTimeout=30 $(echo hi > /hi.txt)`, + "--", "example.com", + "docker system dial-stdio", + }, + }, + { + doc: "bad username", + url: `ssh://$(shutdown)me@example.com`, + expectedCmd: []string{ + "-l", `'$(shutdown)me'`, + "--", "example.com", + "docker system dial-stdio", + }, + }, + { + doc: "bad hostname", + url: `ssh://$(shutdown)example.com`, + expectedCmd: []string{ + "--", `'$(shutdown)example.com'`, + "docker system dial-stdio", + }, + }, + { + doc: "bad path", + url: `ssh://example.com/var/run/docker.sock '$(echo hello > /hello.txt)'`, + expectedCmd: []string{ + "--", "example.com", + `docker "--host=unix:///var/run/docker.sock '\$(echo hello > /hello.txt)'" system dial-stdio`, + }, + }, + { + doc: "missing command", + url: "ssh://example.com", + customCmd: []string{}, + expectedError: "no remote command specified", + }, + } + for _, tc := range testCases { + t.Run(tc.doc, func(t *testing.T) { + sp, err := ParseURL(tc.url) + assert.NilError(t, err) + + var commandAndArgs []string + if tc.customCmd == nil { + socketPath := sp.Path + commandAndArgs = []string{"docker", "system", "dial-stdio"} + if strings.Trim(socketPath, "/") != "" { + commandAndArgs = []string{"docker", "--host=unix://" + socketPath, "system", "dial-stdio"} + } + } + + actualCmd, err := sp.Command(tc.sshFlags, commandAndArgs...) + if tc.expectedError == "" { + assert.NilError(t, err) + assert.Check(t, is.DeepEqual(actualCmd, tc.expectedCmd), "%+#v", actualCmd) + } else { + assert.Check(t, is.Error(err, tc.expectedError)) + assert.Check(t, is.Nil(actualCmd)) + } + }) + } +}