1
0
mirror of https://github.com/docker/cli.git synced 2026-01-06 05:41:44 +03:00

Merge pull request #6147 from thaJeztah/connhelper_quote

cli/connhelper: quote ssh arguments to prevent shell injection
This commit is contained in:
Sebastiaan van Stijn
2025-06-25 17:21:12 +02:00
committed by GitHub
7 changed files with 610 additions and 15 deletions

View File

@@ -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

View File

@@ -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.

View File

@@ -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

View File

@@ -0,0 +1,95 @@
// Copyright (c) 2016, Daniel Martí <mvdan@mvdan.cc>
// 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
}

View File

@@ -0,0 +1,187 @@
// Copyright (c) 2021, Daniel Martí <mvdan@mvdan.cc>
// 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')
}

View File

@@ -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
}

View File

@@ -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))
}
})
}
}