Merge pull request #2955 from thaJeztah/master_context_check

[master] Check contexts before importing them to reduce risk of extracted files escaping context store
This commit is contained in:
Silvin Lubecki 2021-02-02 14:16:59 +01:00 committed by GitHub
commit 70a00157f1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 160 additions and 31 deletions

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

Binary file not shown.

Binary file not shown.