diff --git a/cli/command/context/cmd.go b/cli/command/context/cmd.go index 1b6898456d..6dce68aeaa 100644 --- a/cli/command/context/cmd.go +++ b/cli/command/context/cmd.go @@ -1,10 +1,6 @@ package context import ( - "errors" - "fmt" - "regexp" - "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/spf13/cobra" @@ -30,20 +26,3 @@ func NewContextCommand(dockerCli command.Cli) *cobra.Command { ) return cmd } - -const restrictedNamePattern = "^[a-zA-Z0-9][a-zA-Z0-9_.+-]+$" - -var restrictedNameRegEx = regexp.MustCompile(restrictedNamePattern) - -func validateContextName(name string) error { - if name == "" { - return errors.New("context name cannot be empty") - } - if name == "default" { - return errors.New(`"default" is a reserved context name`) - } - if !restrictedNameRegEx.MatchString(name) { - return fmt.Errorf("context name %q is invalid, names are validated against regexp %q", name, restrictedNamePattern) - } - return nil -} diff --git a/cli/command/context/create.go b/cli/command/context/create.go index 9d5865a70a..9e6fe0295b 100644 --- a/cli/command/context/create.go +++ b/cli/command/context/create.go @@ -137,7 +137,7 @@ func createNewContext(o *CreateOptions, stackOrchestrator command.Orchestrator, } func checkContextNameForCreation(s store.Reader, name string) error { - if err := validateContextName(name); err != nil { + if err := store.ValidateContextName(name); err != nil { return err } if _, err := s.GetMetadata(name); !store.IsErrContextDoesNotExist(err) { diff --git a/cli/command/context/export.go b/cli/command/context/export.go index fd66a5d490..6013071402 100644 --- a/cli/command/context/export.go +++ b/cli/command/context/export.go @@ -77,7 +77,7 @@ func writeTo(dockerCli command.Cli, reader io.Reader, dest string) error { // RunExport exports a Docker context func RunExport(dockerCli command.Cli, opts *ExportOptions) error { - if err := validateContextName(opts.ContextName); err != nil && opts.ContextName != command.DefaultContextName { + if err := store.ValidateContextName(opts.ContextName); err != nil && opts.ContextName != command.DefaultContextName { return err } ctxMeta, err := dockerCli.ContextStore().GetMetadata(opts.ContextName) diff --git a/cli/command/context/update.go b/cli/command/context/update.go index 9165bb3019..3c67fdd207 100644 --- a/cli/command/context/update.go +++ b/cli/command/context/update.go @@ -68,7 +68,7 @@ func newUpdateCommand(dockerCli command.Cli) *cobra.Command { // RunUpdate updates a Docker context func RunUpdate(cli command.Cli, o *UpdateOptions) error { - if err := validateContextName(o.Name); err != nil { + if err := store.ValidateContextName(o.Name); err != nil { return err } s := cli.ContextStore() diff --git a/cli/command/context/use.go b/cli/command/context/use.go index 97e3a97056..a3998da69d 100644 --- a/cli/command/context/use.go +++ b/cli/command/context/use.go @@ -5,6 +5,7 @@ import ( "os" "github.com/docker/cli/cli/command" + "github.com/docker/cli/cli/context/store" "github.com/spf13/cobra" ) @@ -23,7 +24,7 @@ func newUseCommand(dockerCli command.Cli) *cobra.Command { // RunUse set the current Docker context func RunUse(dockerCli command.Cli, name string) error { - if err := validateContextName(name); err != nil && name != "default" { + if err := store.ValidateContextName(name); err != nil && name != "default" { return err } if _, err := dockerCli.ContextStore().GetMetadata(name); err != nil && name != "default" { diff --git a/cli/context/store/store.go b/cli/context/store/store.go index b3cd4c54ed..71220e9ac0 100644 --- a/cli/context/store/store.go +++ b/cli/context/store/store.go @@ -7,19 +7,24 @@ import ( "bytes" _ "crypto/sha256" // ensure ids can be computed "encoding/json" - "errors" "fmt" "io" "io/ioutil" "net/http" "path" "path/filepath" + "regexp" "strings" "github.com/docker/docker/errdefs" digest "github.com/opencontainers/go-digest" + "github.com/pkg/errors" ) +const restrictedNamePattern = "^[a-zA-Z0-9][a-zA-Z0-9_.+-]+$" + +var restrictedNameRegEx = regexp.MustCompile(restrictedNamePattern) + // Store provides a context store for easily remembering endpoints configuration type Store interface { Reader @@ -184,6 +189,20 @@ func (s *store) GetStorageInfo(contextName string) StorageInfo { } } +// ValidateContextName checks a context name is valid. +func ValidateContextName(name string) error { + if name == "" { + return errors.New("context name cannot be empty") + } + if name == "default" { + return errors.New(`"default" is a reserved context name`) + } + if !restrictedNameRegEx.MatchString(name) { + return fmt.Errorf("context name %q is invalid, names are validated against regexp %q", name, restrictedNamePattern) + } + return nil +} + // Export exports an existing namespace into an opaque data stream // This stream is actually a tarball containing context metadata and TLS materials, but it does // not map 1:1 the layout of the context store (don't try to restore it manually without calling store.Import) @@ -295,6 +314,19 @@ func Import(name string, s Writer, reader io.Reader) error { } } +func isValidFilePath(p string) error { + if p != metaFile && !strings.HasPrefix(p, "tls/") { + return errors.New("unexpected context file") + } + if path.Clean(p) != p { + return errors.New("unexpected path format") + } + if strings.Contains(p, `\`) { + return errors.New(`unexpected '\' in path`) + } + return nil +} + func importTar(name string, s Writer, reader io.Reader) error { tr := tar.NewReader(&LimitedReader{R: reader, N: maxAllowedFileSizeToImport}) tlsData := ContextTLSData{ @@ -309,10 +341,13 @@ func importTar(name string, s Writer, reader io.Reader) error { if err != nil { return err } - if hdr.Typeflag == tar.TypeDir { + if hdr.Typeflag != tar.TypeReg { // skip this entry, only taking files into account continue } + if err := isValidFilePath(hdr.Name); err != nil { + return errors.Wrap(err, hdr.Name) + } if hdr.Name == metaFile { data, err := ioutil.ReadAll(tr) if err != nil { @@ -358,10 +393,13 @@ func importZip(name string, s Writer, reader io.Reader) error { var importedMetaFile bool for _, zf := range zr.File { fi := zf.FileInfo() - if fi.IsDir() { - // skip this entry, only taking files into account + if !fi.Mode().IsRegular() { + // skip this entry, only taking regular files into account continue } + if err := isValidFilePath(zf.Name); err != nil { + return errors.Wrap(err, zf.Name) + } if zf.Name == metaFile { f, err := zf.Open() if err != nil { @@ -408,6 +446,9 @@ func parseMetadata(data []byte, name string) (Metadata, error) { if err := json.Unmarshal(data, &meta); err != nil { return meta, err } + if err := ValidateContextName(name); err != nil { + return Metadata{}, err + } meta.Name = name return meta, nil } diff --git a/cli/context/store/store_test.go b/cli/context/store/store_test.go index df506061e5..4b412652de 100644 --- a/cli/context/store/store_test.go +++ b/cli/context/store/store_test.go @@ -175,7 +175,7 @@ func TestImportTarInvalid(t *testing.T) { var r io.Reader = source s := New(testDir, testCfg) err = Import("tarInvalid", s, r) - assert.ErrorContains(t, err, "invalid context: no metadata found") + assert.ErrorContains(t, err, "unexpected context file") } func TestImportZip(t *testing.T) { @@ -254,5 +254,5 @@ func TestImportZipInvalid(t *testing.T) { var r io.Reader = source s := New(testDir, testCfg) err = Import("zipInvalid", s, r) - assert.ErrorContains(t, err, "invalid context: no metadata found") + assert.ErrorContains(t, err, "unexpected context file") } diff --git a/cli/context/store/storeconfig_test.go b/cli/context/store/storeconfig_test.go index 7f59e3f015..4d9a3b63fb 100644 --- a/cli/context/store/storeconfig_test.go +++ b/cli/context/store/storeconfig_test.go @@ -29,3 +29,32 @@ func TestConfigModification(t *testing.T) { assert.Equal(t, &testEP2{}, cfgCopy.endpointTypes["ep1"]()) assert.Equal(t, &testEP3{}, cfgCopy.endpointTypes["ep2"]()) } + +func TestValidFilePaths(t *testing.T) { + paths := map[string]bool{ + "tls/_/../../something": false, + "tls/../../something": false, + "../../something": false, + "/tls/absolute/unix/path": false, + `C:\tls\absolute\windows\path`: false, + "C:/tls/absolute/windows/path": false, + "tls/kubernetes/key.pem": true, + } + for p, expectedValid := range paths { + err := isValidFilePath(p) + assert.Equal(t, err == nil, expectedValid, "%q should report valid as: %v", p, expectedValid) + } +} + +func TestValidateContextName(t *testing.T) { + names := map[string]bool{ + "../../invalid/escape": false, + "/invalid/absolute": false, + `\invalid\windows`: false, + "validname": true, + } + for n, expectedValid := range names { + err := ValidateContextName(n) + assert.Equal(t, err == nil, expectedValid, "%q should report valid as: %v", n, expectedValid) + } +} diff --git a/e2e/context/context_test.go b/e2e/context/context_test.go index ce22f7b7c3..6212b98dea 100644 --- a/e2e/context/context_test.go +++ b/e2e/context/context_test.go @@ -1,8 +1,11 @@ package context import ( + "io/ioutil" + "os" "testing" + "gotest.tools/v3/assert" "gotest.tools/v3/golden" "gotest.tools/v3/icmd" ) @@ -19,3 +22,73 @@ func TestContextList(t *testing.T) { }) golden.Assert(t, result.Stdout(), "context-ls.golden") } + +func TestContextImportNoTLS(t *testing.T) { + d, _ := ioutil.TempDir("", "") + defer func() { + os.RemoveAll(d) + }() + cmd := icmd.Command("docker", "context", "import", "remote", "./testdata/test-dockerconfig.tar") + cmd.Env = append(cmd.Env, + "DOCKER_CONFIG="+d, + ) + icmd.RunCmd(cmd).Assert(t, icmd.Success) + + cmd = icmd.Command("docker", "context", "ls") + cmd.Env = append(cmd.Env, + "DOCKER_CONFIG="+d, + "KUBECONFIG=./testdata/test-kubeconfig", // Allows reuse of context-ls.golden + ) + result := icmd.RunCmd(cmd).Assert(t, icmd.Success) + golden.Assert(t, result.Stdout(), "context-ls.golden") +} + +func TestContextImportTLS(t *testing.T) { + d, _ := ioutil.TempDir("", "") + defer func() { + os.RemoveAll(d) + }() + cmd := icmd.Command("docker", "context", "import", "test", "./testdata/test-dockerconfig-tls.tar") + cmd.Env = append(cmd.Env, + "DOCKER_CONFIG="+d, + ) + icmd.RunCmd(cmd).Assert(t, icmd.Success) + + cmd = icmd.Command("docker", "context", "ls") + cmd.Env = append(cmd.Env, + "DOCKER_CONFIG="+d, + ) + result := icmd.RunCmd(cmd).Assert(t, icmd.Success) + golden.Assert(t, result.Stdout(), "context-ls-tls.golden") + + b, err := ioutil.ReadFile(d + "/contexts/tls/9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08/kubernetes/key.pem") + assert.NilError(t, err) + assert.Equal(t, string(b), `-----BEGIN RSA PRIVATE KEY----- +MIIEpAIBAAKCAQEArQk77K5sgrQYY6HiQ1y7AC+67HrRB36oEvR+Fq60RsFcc3cZ +xAvMkRSBPjQyskjdYY7kfykGHhfJxGKopb3cDJx3eDBxjgAniwnnOMmHVWbf8Eik +o0sNxkgzQPGq83nL3QvVxm3xgqe4nlTdR/Swoq6Pv0oaVYvPPMnaZIF89SJ/wlNT +myCs6Uq00dICi20II+M2Nw9b+EVEK4ENl+SlrsK7iuoBIh/H0ZghxOthO9J/HeBb +hmM4wcs1OonhPDYKHEaChYA7/Q3/8OBp3bAdlQJ1ziyP3ROAKHL2NwwkGZ8o8HP8 +u0ex/NAb8w5J5WNePqYQd/sqfisfNpA5VIKcEQIDAQABAoIBABLo4W2aGi2mdMve +kxV9esoobSsOuO0ywDdiFK1x5i2dT/cmWuB70Z1BOmaL2cZ2BAt3TC1BVHPRcbFO +ftOuDfAq4Tt3P9Ge3rNpH6WrEGka1voxVhyqRRUYKtG8F0yIUOkVNAV9WllG7vwO +ligY63y7yuXCuWID51/jR0SYiglXz6G4gcJKFXtugXXiLUIg08GVWkwOsrACC+hR +mhcHly1926VhN5+ozjNU/GZ1LaTuK6erBZakH5bqlN97s5rrk0ZRwk/JtnkoRRdI +cq0918Za2vqGDHZ3MqLttL52YfDXPIEJPwlFdvC/+sXK2NhUB/xY4yuliU3sY0sf +XsIvIWECgYEAwD8AnZI0hnGv8hc6zJppHFRwhrtLZ+09SJwPv5Y4wxuuk5dzNkpf +xCNo5hjSVYA1MMmWG8p/sEXo2IyCT8sWDNCn9kieTXihxRxbj88Y2qA5O4N46Zy4 +kPngjkP5PPDMkwaQQgUr9LvlWS7P6OJkH18ZN8s3QhMaKcHu9FFT44UCgYEA5mte +mMSDf9hUK3IK+yrGX62qc2H+ecXN3Zf3nehyiz+dX4ZXhBwBkwJ/mHvuAZPfoFUN +Xg6cdyWFJg9ynm45JXnDjmYPGmFLn0mP3Mje/+SbbW2fdFWHJW/maqj4uUqqgQd+ +pGNzKXq34MzDrpsqIJ7AHu3LYVMOoLAVqC7LXh0CgYEAnLF9ZfFqQH7fgvouIeBl +dgLZKOf2AUJcJheVunnN0DF67K+P55tdTTfzY0CuB6SVNivI3uQBiYKh1AdKm5ET +auSTUmlEJi8B4/BGLQQG5QOdQoXZgsgLo5cX0b1To7k9dUTvRfCDMFoKCNPgAJiu +NOfFXTWU15VMSObaRmcXciUCgYEA5e1cXwsxwUAodZX+eTXs8ArHHQ47Nl55GFeN +wufybRuUuX7AE9cyhvUmSA3aqX5a144noaTo40fwftNJZ+jLY6cGyjDzfzp5kMCC +KynSxPzlUCPkytyR2Hy6K9LjJ1rnm4vUBswqXcjUdiE+Xxz8w8JGKlbV7Q9JeHVd +lw7i5s0CgYAn9T9ySI3xCbrUa/XV/ZY2hopUdH5CDPeTd2eH+L+lctkD9nlzLrpj +qij+jaEUweymNx0uttgv02J3DYcIIvVq3RNAwORy5Mp9KasHmjbW2xq+HAq5yFOO +1ma82F5zeUl+bKqjMRCY8IVZ349VxRZtb2RVVEKyVswb7HmKp6gGbA== +-----END RSA PRIVATE KEY----- +`) +} diff --git a/e2e/context/testdata/context-ls-notls.golden b/e2e/context/testdata/context-ls-notls.golden new file mode 100644 index 0000000000..1747503305 --- /dev/null +++ b/e2e/context/testdata/context-ls-notls.golden @@ -0,0 +1,3 @@ +NAME DESCRIPTION DOCKER ENDPOINT KUBERNETES ENDPOINT ORCHESTRATOR +default * Current DOCKER_HOST based configuration unix:///var/run/docker.sock swarm +remote my remote cluster ssh://someserver https://someserver (default) kubernetes diff --git a/e2e/context/testdata/context-ls-tls.golden b/e2e/context/testdata/context-ls-tls.golden new file mode 100644 index 0000000000..f0f0d464e7 --- /dev/null +++ b/e2e/context/testdata/context-ls-tls.golden @@ -0,0 +1,3 @@ +NAME DESCRIPTION DOCKER ENDPOINT KUBERNETES ENDPOINT ORCHESTRATOR +default * Current DOCKER_HOST based configuration unix:///var/run/docker.sock swarm +test unix:///var/run/docker.sock https://kubernetes.docker.internal:6443 (default) swarm diff --git a/e2e/context/testdata/test-dockerconfig-tls.tar b/e2e/context/testdata/test-dockerconfig-tls.tar new file mode 100644 index 0000000000..6dc21a6014 Binary files /dev/null and b/e2e/context/testdata/test-dockerconfig-tls.tar differ diff --git a/e2e/context/testdata/test-dockerconfig.tar b/e2e/context/testdata/test-dockerconfig.tar new file mode 100644 index 0000000000..92eab23bda Binary files /dev/null and b/e2e/context/testdata/test-dockerconfig.tar differ