Skip to content

Commit 70a0015

Browse files
silvin-lubeckisilvin-lubecki
authored
Merge pull request #2955 from thaJeztah/master_context_check
[master] Check contexts before importing them to reduce risk of extracted files escaping context store
2 parents a22ed24 + b43b852 commit 70a0015

13 files changed

Lines changed: 160 additions & 31 deletions

File tree

‎cli/command/context/cmd.go‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
package context
22

33
import (
4-
"errors"
5-
"fmt"
6-
"regexp"
7-
84
"github.com/docker/cli/cli"
95
"github.com/docker/cli/cli/command"
106
"github.com/spf13/cobra"
@@ -30,20 +26,3 @@ func NewContextCommand(dockerCli command.Cli) *cobra.Command {
3026
)
3127
return cmd
3228
}
33-
34-
const restrictedNamePattern = "^[a-zA-Z0-9][a-zA-Z0-9_.+-]+$"
35-
36-
var restrictedNameRegEx = regexp.MustCompile(restrictedNamePattern)
37-
38-
func validateContextName(name string) error {
39-
if name == "" {
40-
return errors.New("context name cannot be empty")
41-
}
42-
if name == "default" {
43-
return errors.New(`"default" is a reserved context name`)
44-
}
45-
if !restrictedNameRegEx.MatchString(name) {
46-
return fmt.Errorf("context name %q is invalid, names are validated against regexp %q", name, restrictedNamePattern)
47-
}
48-
return nil
49-
}

‎cli/command/context/create.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ func createNewContext(o *CreateOptions, stackOrchestrator command.Orchestrator,
137137
}
138138

139139
func checkContextNameForCreation(s store.Reader, name string) error {
140-
if err := validateContextName(name); err != nil {
140+
if err := store.ValidateContextName(name); err != nil {
141141
return err
142142
}
143143
if _, err := s.GetMetadata(name); !store.IsErrContextDoesNotExist(err) {

‎cli/command/context/export.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ func writeTo(dockerCli command.Cli, reader io.Reader, dest string) error {
7777

7878
// RunExport exports a Docker context
7979
func RunExport(dockerCli command.Cli, opts *ExportOptions) error {
80-
if err := validateContextName(opts.ContextName); err != nil && opts.ContextName != command.DefaultContextName {
80+
if err := store.ValidateContextName(opts.ContextName); err != nil && opts.ContextName != command.DefaultContextName {
8181
return err
8282
}
8383
ctxMeta, err := dockerCli.ContextStore().GetMetadata(opts.ContextName)

‎cli/command/context/update.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func newUpdateCommand(dockerCli command.Cli) *cobra.Command {
6868

6969
// RunUpdate updates a Docker context
7070
func RunUpdate(cli command.Cli, o *UpdateOptions) error {
71-
if err := validateContextName(o.Name); err != nil {
71+
if err := store.ValidateContextName(o.Name); err != nil {
7272
return err
7373
}
7474
s := cli.ContextStore()

‎cli/command/context/use.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"os"
66

77
"github.com/docker/cli/cli/command"
8+
"github.com/docker/cli/cli/context/store"
89
"github.com/spf13/cobra"
910
)
1011

@@ -23,7 +24,7 @@ func newUseCommand(dockerCli command.Cli) *cobra.Command {
2324

2425
// RunUse set the current Docker context
2526
func RunUse(dockerCli command.Cli, name string) error {
26-
if err := validateContextName(name); err != nil && name != "default" {
27+
if err := store.ValidateContextName(name); err != nil && name != "default" {
2728
return err
2829
}
2930
if _, err := dockerCli.ContextStore().GetMetadata(name); err != nil && name != "default" {

‎cli/context/store/store.go‎

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,24 @@ import (
77
"bytes"
88
_ "crypto/sha256" // ensure ids can be computed
99
"encoding/json"
10-
"errors"
1110
"fmt"
1211
"io"
1312
"io/ioutil"
1413
"net/http"
1514
"path"
1615
"path/filepath"
16+
"regexp"
1717
"strings"
1818

1919
"github.com/docker/docker/errdefs"
2020
digest "github.com/opencontainers/go-digest"
21+
"github.com/pkg/errors"
2122
)
2223

24+
const restrictedNamePattern = "^[a-zA-Z0-9][a-zA-Z0-9_.+-]+$"
25+
26+
var restrictedNameRegEx = regexp.MustCompile(restrictedNamePattern)
27+
2328
// Store provides a context store for easily remembering endpoints configuration
2429
type Store interface {
2530
Reader
@@ -184,6 +189,20 @@ func (s *store) GetStorageInfo(contextName string) StorageInfo {
184189
}
185190
}
186191

192+
// ValidateContextName checks a context name is valid.
193+
func ValidateContextName(name string) error {
194+
if name == "" {
195+
return errors.New("context name cannot be empty")
196+
}
197+
if name == "default" {
198+
return errors.New(`"default" is a reserved context name`)
199+
}
200+
if !restrictedNameRegEx.MatchString(name) {
201+
return fmt.Errorf("context name %q is invalid, names are validated against regexp %q", name, restrictedNamePattern)
202+
}
203+
return nil
204+
}
205+
187206
// Export exports an existing namespace into an opaque data stream
188207
// This stream is actually a tarball containing context metadata and TLS materials, but it does
189208
// 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 {
295314
}
296315
}
297316

317+
func isValidFilePath(p string) error {
318+
if p != metaFile && !strings.HasPrefix(p, "tls/") {
319+
return errors.New("unexpected context file")
320+
}
321+
if path.Clean(p) != p {
322+
return errors.New("unexpected path format")
323+
}
324+
if strings.Contains(p, `\`) {
325+
return errors.New(`unexpected '\' in path`)
326+
}
327+
return nil
328+
}
329+
298330
func importTar(name string, s Writer, reader io.Reader) error {
299331
tr := tar.NewReader(&LimitedReader{R: reader, N: maxAllowedFileSizeToImport})
300332
tlsData := ContextTLSData{
@@ -309,10 +341,13 @@ func importTar(name string, s Writer, reader io.Reader) error {
309341
if err != nil {
310342
return err
311343
}
312-
if hdr.Typeflag == tar.TypeDir {
344+
if hdr.Typeflag != tar.TypeReg {
313345
// skip this entry, only taking files into account
314346
continue
315347
}
348+
if err := isValidFilePath(hdr.Name); err != nil {
349+
return errors.Wrap(err, hdr.Name)
350+
}
316351
if hdr.Name == metaFile {
317352
data, err := ioutil.ReadAll(tr)
318353
if err != nil {
@@ -358,10 +393,13 @@ func importZip(name string, s Writer, reader io.Reader) error {
358393
var importedMetaFile bool
359394
for _, zf := range zr.File {
360395
fi := zf.FileInfo()
361-
if fi.IsDir() {
362-
// skip this entry, only taking files into account
396+
if !fi.Mode().IsRegular() {
397+
// skip this entry, only taking regular files into account
363398
continue
364399
}
400+
if err := isValidFilePath(zf.Name); err != nil {
401+
return errors.Wrap(err, zf.Name)
402+
}
365403
if zf.Name == metaFile {
366404
f, err := zf.Open()
367405
if err != nil {
@@ -408,6 +446,9 @@ func parseMetadata(data []byte, name string) (Metadata, error) {
408446
if err := json.Unmarshal(data, &meta); err != nil {
409447
return meta, err
410448
}
449+
if err := ValidateContextName(name); err != nil {
450+
return Metadata{}, err
451+
}
411452
meta.Name = name
412453
return meta, nil
413454
}

‎cli/context/store/store_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ func TestImportTarInvalid(t *testing.T) {
175175
var r io.Reader = source
176176
s := New(testDir, testCfg)
177177
err = Import("tarInvalid", s, r)
178-
assert.ErrorContains(t, err, "invalid context: no metadata found")
178+
assert.ErrorContains(t, err, "unexpected context file")
179179
}
180180

181181
func TestImportZip(t *testing.T) {
@@ -254,5 +254,5 @@ func TestImportZipInvalid(t *testing.T) {
254254
var r io.Reader = source
255255
s := New(testDir, testCfg)
256256
err = Import("zipInvalid", s, r)
257-
assert.ErrorContains(t, err, "invalid context: no metadata found")
257+
assert.ErrorContains(t, err, "unexpected context file")
258258
}

‎cli/context/store/storeconfig_test.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,32 @@ func TestConfigModification(t *testing.T) {
2929
assert.Equal(t, &testEP2{}, cfgCopy.endpointTypes["ep1"]())
3030
assert.Equal(t, &testEP3{}, cfgCopy.endpointTypes["ep2"]())
3131
}
32+
33+
func TestValidFilePaths(t *testing.T) {
34+
paths := map[string]bool{
35+
"tls/_/../../something": false,
36+
"tls/../../something": false,
37+
"../../something": false,
38+
"/tls/absolute/unix/path": false,
39+
`C:\tls\absolute\windows\path`: false,
40+
"C:/tls/absolute/windows/path": false,
41+
"tls/kubernetes/key.pem": true,
42+
}
43+
for p, expectedValid := range paths {
44+
err := isValidFilePath(p)
45+
assert.Equal(t, err == nil, expectedValid, "%q should report valid as: %v", p, expectedValid)
46+
}
47+
}
48+
49+
func TestValidateContextName(t *testing.T) {
50+
names := map[string]bool{
51+
"../../invalid/escape": false,
52+
"/invalid/absolute": false,
53+
`\invalid\windows`: false,
54+
"validname": true,
55+
}
56+
for n, expectedValid := range names {
57+
err := ValidateContextName(n)
58+
assert.Equal(t, err == nil, expectedValid, "%q should report valid as: %v", n, expectedValid)
59+
}
60+
}

‎e2e/context/context_test.go‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
package context
22

33
import (
4+
"io/ioutil"
5+
"os"
46
"testing"
57

8+
"gotest.tools/v3/assert"
69
"gotest.tools/v3/golden"
710
"gotest.tools/v3/icmd"
811
)
@@ -19,3 +22,73 @@ func TestContextList(t *testing.T) {
1922
})
2023
golden.Assert(t, result.Stdout(), "context-ls.golden")
2124
}
25+
26+
func TestContextImportNoTLS(t *testing.T) {
27+
d, _ := ioutil.TempDir("", "")
28+
defer func() {
29+
os.RemoveAll(d)
30+
}()
31+
cmd := icmd.Command("docker", "context", "import", "remote", "./testdata/test-dockerconfig.tar")
32+
cmd.Env = append(cmd.Env,
33+
"DOCKER_CONFIG="+d,
34+
)
35+
icmd.RunCmd(cmd).Assert(t, icmd.Success)
36+
37+
cmd = icmd.Command("docker", "context", "ls")
38+
cmd.Env = append(cmd.Env,
39+
"DOCKER_CONFIG="+d,
40+
"KUBECONFIG=./testdata/test-kubeconfig", // Allows reuse of context-ls.golden
41+
)
42+
result := icmd.RunCmd(cmd).Assert(t, icmd.Success)
43+
golden.Assert(t, result.Stdout(), "context-ls.golden")
44+
}
45+
46+
func TestContextImportTLS(t *testing.T) {
47+
d, _ := ioutil.TempDir("", "")
48+
defer func() {
49+
os.RemoveAll(d)
50+
}()
51+
cmd := icmd.Command("docker", "context", "import", "test", "./testdata/test-dockerconfig-tls.tar")
52+
cmd.Env = append(cmd.Env,
53+
"DOCKER_CONFIG="+d,
54+
)
55+
icmd.RunCmd(cmd).Assert(t, icmd.Success)
56+
57+
cmd = icmd.Command("docker", "context", "ls")
58+
cmd.Env = append(cmd.Env,
59+
"DOCKER_CONFIG="+d,
60+
)
61+
result := icmd.RunCmd(cmd).Assert(t, icmd.Success)
62+
golden.Assert(t, result.Stdout(), "context-ls-tls.golden")
63+
64+
b, err := ioutil.ReadFile(d + "/contexts/tls/9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08/kubernetes/key.pem")
65+
assert.NilError(t, err)
66+
assert.Equal(t, string(b), `-----BEGIN RSA PRIVATE KEY-----
67+
MIIEpAIBAAKCAQEArQk77K5sgrQYY6HiQ1y7AC+67HrRB36oEvR+Fq60RsFcc3cZ
68+
xAvMkRSBPjQyskjdYY7kfykGHhfJxGKopb3cDJx3eDBxjgAniwnnOMmHVWbf8Eik
69+
o0sNxkgzQPGq83nL3QvVxm3xgqe4nlTdR/Swoq6Pv0oaVYvPPMnaZIF89SJ/wlNT
70+
myCs6Uq00dICi20II+M2Nw9b+EVEK4ENl+SlrsK7iuoBIh/H0ZghxOthO9J/HeBb
71+
hmM4wcs1OonhPDYKHEaChYA7/Q3/8OBp3bAdlQJ1ziyP3ROAKHL2NwwkGZ8o8HP8
72+
u0ex/NAb8w5J5WNePqYQd/sqfisfNpA5VIKcEQIDAQABAoIBABLo4W2aGi2mdMve
73+
kxV9esoobSsOuO0ywDdiFK1x5i2dT/cmWuB70Z1BOmaL2cZ2BAt3TC1BVHPRcbFO
74+
ftOuDfAq4Tt3P9Ge3rNpH6WrEGka1voxVhyqRRUYKtG8F0yIUOkVNAV9WllG7vwO
75+
ligY63y7yuXCuWID51/jR0SYiglXz6G4gcJKFXtugXXiLUIg08GVWkwOsrACC+hR
76+
mhcHly1926VhN5+ozjNU/GZ1LaTuK6erBZakH5bqlN97s5rrk0ZRwk/JtnkoRRdI
77+
cq0918Za2vqGDHZ3MqLttL52YfDXPIEJPwlFdvC/+sXK2NhUB/xY4yuliU3sY0sf
78+
XsIvIWECgYEAwD8AnZI0hnGv8hc6zJppHFRwhrtLZ+09SJwPv5Y4wxuuk5dzNkpf
79+
xCNo5hjSVYA1MMmWG8p/sEXo2IyCT8sWDNCn9kieTXihxRxbj88Y2qA5O4N46Zy4
80+
kPngjkP5PPDMkwaQQgUr9LvlWS7P6OJkH18ZN8s3QhMaKcHu9FFT44UCgYEA5mte
81+
mMSDf9hUK3IK+yrGX62qc2H+ecXN3Zf3nehyiz+dX4ZXhBwBkwJ/mHvuAZPfoFUN
82+
Xg6cdyWFJg9ynm45JXnDjmYPGmFLn0mP3Mje/+SbbW2fdFWHJW/maqj4uUqqgQd+
83+
pGNzKXq34MzDrpsqIJ7AHu3LYVMOoLAVqC7LXh0CgYEAnLF9ZfFqQH7fgvouIeBl
84+
dgLZKOf2AUJcJheVunnN0DF67K+P55tdTTfzY0CuB6SVNivI3uQBiYKh1AdKm5ET
85+
auSTUmlEJi8B4/BGLQQG5QOdQoXZgsgLo5cX0b1To7k9dUTvRfCDMFoKCNPgAJiu
86+
NOfFXTWU15VMSObaRmcXciUCgYEA5e1cXwsxwUAodZX+eTXs8ArHHQ47Nl55GFeN
87+
wufybRuUuX7AE9cyhvUmSA3aqX5a144noaTo40fwftNJZ+jLY6cGyjDzfzp5kMCC
88+
KynSxPzlUCPkytyR2Hy6K9LjJ1rnm4vUBswqXcjUdiE+Xxz8w8JGKlbV7Q9JeHVd
89+
lw7i5s0CgYAn9T9ySI3xCbrUa/XV/ZY2hopUdH5CDPeTd2eH+L+lctkD9nlzLrpj
90+
qij+jaEUweymNx0uttgv02J3DYcIIvVq3RNAwORy5Mp9KasHmjbW2xq+HAq5yFOO
91+
1ma82F5zeUl+bKqjMRCY8IVZ349VxRZtb2RVVEKyVswb7HmKp6gGbA==
92+
-----END RSA PRIVATE KEY-----
93+
`)
94+
}

‎e2e/context/testdata/context-ls-notls.golden‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
NAME DESCRIPTION DOCKER ENDPOINT KUBERNETES ENDPOINT ORCHESTRATOR
2+
default * Current DOCKER_HOST based configuration unix:///var/run/docker.sock swarm
3+
remote my remote cluster ssh://someserver https://someserver (default) kubernetes

0 commit comments

Comments
 (0)