Browse Source

Merge pull request #17213 from calavera/volume_driver_validate_name

Validate volume names for the local driver.
David Calavera 9 years ago
parent
commit
8247eff8a8
5 changed files with 85 additions and 5 deletions
  1. 2 3
      daemon/daemon.go
  2. 8 0
      errors/daemon.go
  3. 9 0
      utils/names.go
  4. 21 2
      volume/local/local.go
  5. 45 0
      volume/local/local_test.go

+ 2 - 3
daemon/daemon.go

@@ -12,7 +12,6 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
-	"regexp"
 	"runtime"
 	"strings"
 	"sync"
@@ -59,8 +58,8 @@ import (
 )
 
 var (
-	validContainerNameChars   = `[a-zA-Z0-9][a-zA-Z0-9_.-]`
-	validContainerNamePattern = regexp.MustCompile(`^/?` + validContainerNameChars + `+$`)
+	validContainerNameChars   = utils.RestrictedNameChars
+	validContainerNamePattern = utils.RestrictedNamePattern
 
 	errSystemNotSupported = errors.New("The Docker daemon is not supported on this platform.")
 )

+ 8 - 0
errors/daemon.go

@@ -385,6 +385,14 @@ var (
 		HTTPStatusCode: http.StatusInternalServerError,
 	})
 
+	// ErrorCodeVolumeName is generated when the name of named volume isn't valid.
+	ErrorCodeVolumeName = errcode.Register(errGroup, errcode.ErrorDescriptor{
+		Value:          "VOLUME_NAME_INVALID",
+		Message:        "%s includes invalid characters for a local volume name, only %s are allowed",
+		Description:    "The name of volume is invalid",
+		HTTPStatusCode: http.StatusBadRequest,
+	})
+
 	// ErrorCodeVolumeFromBlank is generated when path to a volume is blank.
 	ErrorCodeVolumeFromBlank = errcode.Register(errGroup, errcode.ErrorDescriptor{
 		Value:          "VOLUMEFROMBLANK",

+ 9 - 0
utils/names.go

@@ -0,0 +1,9 @@
+package utils
+
+import "regexp"
+
+// RestrictedNameChars collects the characters allowed to represent a name, normally used to validate container and volume names.
+const RestrictedNameChars = `[a-zA-Z0-9][a-zA-Z0-9_.-]`
+
+// RestrictedNamePattern is a regular expression to validate names against the collection of restricted characters.
+var RestrictedNamePattern = regexp.MustCompile(`^/?` + RestrictedNameChars + `+$`)

+ 21 - 2
volume/local/local.go

@@ -11,7 +11,9 @@ import (
 	"path/filepath"
 	"sync"
 
+	derr "github.com/docker/docker/errors"
 	"github.com/docker/docker/pkg/idtools"
+	"github.com/docker/docker/utils"
 	"github.com/docker/docker/volume"
 )
 
@@ -23,8 +25,14 @@ const (
 	volumesPathName    = "volumes"
 )
 
-// ErrNotFound is the typed error returned when the requested volume name can't be found
-var ErrNotFound = errors.New("volume not found")
+var (
+	// ErrNotFound is the typed error returned when the requested volume name can't be found
+	ErrNotFound = errors.New("volume not found")
+	// volumeNameRegex ensures the name asigned for the volume is valid.
+	// This name is used to create the bind directory, so we need to avoid characters that
+	// would make the path to escape the root directory.
+	volumeNameRegex = utils.RestrictedNamePattern
+)
 
 // New instantiates a new Root instance with the provided scope. Scope
 // is the base path that the Root instance uses to store its
@@ -96,6 +104,10 @@ func (r *Root) Name() string {
 // the underlying directory tree required for this volume in the
 // process.
 func (r *Root) Create(name string, _ map[string]string) (volume.Volume, error) {
+	if err := r.validateName(name); err != nil {
+		return nil, err
+	}
+
 	r.m.Lock()
 	defer r.m.Unlock()
 
@@ -174,6 +186,13 @@ func (r *Root) Get(name string) (volume.Volume, error) {
 	return v, nil
 }
 
+func (r *Root) validateName(name string) error {
+	if !volumeNameRegex.MatchString(name) {
+		return derr.ErrorCodeVolumeName.WithArgs(name, utils.RestrictedNameChars)
+	}
+	return nil
+}
+
 // localVolume implements the Volume interface from the volume package and
 // represents the volumes created by Root.
 type localVolume struct {

+ 45 - 0
volume/local/local_test.go

@@ -79,3 +79,48 @@ func TestInitializeWithVolumes(t *testing.T) {
 		t.Fatal("expected to re-initialize root with existing volumes")
 	}
 }
+
+func TestCreate(t *testing.T) {
+	rootDir, err := ioutil.TempDir("", "local-volume-test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(rootDir)
+
+	r, err := New(rootDir, 0, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	cases := map[string]bool{
+		"name":                  true,
+		"name-with-dash":        true,
+		"name_with_underscore":  true,
+		"name/with/slash":       false,
+		"name/with/../../slash": false,
+		"./name":                false,
+		"../name":               false,
+		"./":                    false,
+		"../":                   false,
+		"~":                     false,
+		".":                     false,
+		"..":                    false,
+		"...":                   false,
+	}
+
+	for name, success := range cases {
+		v, err := r.Create(name, nil)
+		if success {
+			if err != nil {
+				t.Fatal(err)
+			}
+			if v.Name() != name {
+				t.Fatalf("Expected volume with name %s, got %s", name, v.Name())
+			}
+		} else {
+			if err == nil {
+				t.Fatalf("Expected error creating volume with name %s, got nil", name)
+			}
+		}
+	}
+}