Browse code

Move volume name validation to the local driver.

Delegate validation tasks to the volume drivers. It's up to them
to decide whether a name is valid or not.
Restrict volume names for the local driver to prevent creating
mount points outside docker's volumes directory.

Signed-off-by: David Calavera <david.calavera@gmail.com>

David Calavera authored on 2015/10/21 02:09:48
Showing 7 changed files
... ...
@@ -12,7 +12,6 @@ import (
12 12
 	"io/ioutil"
13 13
 	"os"
14 14
 	"path/filepath"
15
-	"regexp"
16 15
 	"runtime"
17 16
 	"strings"
18 17
 	"sync"
... ...
@@ -59,8 +58,8 @@ import (
59 59
 )
60 60
 
61 61
 var (
62
-	validContainerNameChars   = `[a-zA-Z0-9][a-zA-Z0-9_.-]`
63
-	validContainerNamePattern = regexp.MustCompile(`^/?` + validContainerNameChars + `+$`)
62
+	validContainerNameChars   = utils.RestrictedNameChars
63
+	validContainerNamePattern = utils.RestrictedNamePattern
64 64
 
65 65
 	errSystemNotSupported = errors.New("The Docker daemon is not supported on this platform.")
66 66
 )
... ...
@@ -24,10 +24,6 @@ func TestParseBindMount(t *testing.T) {
24 24
 		{"name:/tmp:ro", "local", "/tmp", "", "name", "local", false, false},
25 25
 		{"local/name:/tmp:rw", "", "/tmp", "", "local/name", "local", true, false},
26 26
 		{"/tmp:tmp", "", "", "", "", "", true, true},
27
-		{"./name:/tmp", "", "", "", "", "", true, true},
28
-		{"../name:/tmp", "", "", "", "", "", true, true},
29
-		{"./:/tmp", "", "", "", "", "", true, true},
30
-		{"../:/tmp", "", "", "", "", "", true, true},
31 27
 	}
32 28
 
33 29
 	for _, c := range cases {
... ...
@@ -6,7 +6,6 @@ import (
6 6
 	"io/ioutil"
7 7
 	"os"
8 8
 	"path/filepath"
9
-	"regexp"
10 9
 	"sort"
11 10
 	"strings"
12 11
 
... ...
@@ -103,12 +102,6 @@ func parseBindMount(spec, volumeDriver string) (*mountPoint, error) {
103 103
 	}
104 104
 
105 105
 	if len(source) == 0 {
106
-		//validate the name of named volume
107
-		nameRegex := regexp.MustCompile(`(^.+[^0-9A-Za-z_]+$)|(/)`)
108
-		if nameRegex.MatchString(name) {
109
-			return nil, derr.ErrorCodeVolumeName.WithArgs(name)
110
-		}
111
-
112 106
 		bind.Driver = volumeDriver
113 107
 		if len(bind.Driver) == 0 {
114 108
 			bind.Driver = volume.DefaultDriverName
... ...
@@ -387,10 +387,10 @@ var (
387 387
 
388 388
 	// ErrorCodeVolumeName is generated when the name of named volume isn't valid.
389 389
 	ErrorCodeVolumeName = errcode.Register(errGroup, errcode.ErrorDescriptor{
390
-		Value:          "VOLUMENAME",
391
-		Message:        "%s looks like a relative path, but it's taken as a name. And it is not a valid name.",
392
-		Description:    "The name of named volume is invalid",
393
-		HTTPStatusCode: http.StatusInternalServerError,
390
+		Value:          "VOLUME_NAME_INVALID",
391
+		Message:        "%s includes invalid characters for a local volume name, only %s are allowed",
392
+		Description:    "The name of volume is invalid",
393
+		HTTPStatusCode: http.StatusBadRequest,
394 394
 	})
395 395
 
396 396
 	// ErrorCodeVolumeFromBlank is generated when path to a volume is blank.
397 397
new file mode 100644
... ...
@@ -0,0 +1,9 @@
0
+package utils
1
+
2
+import "regexp"
3
+
4
+// RestrictedNameChars collects the characters allowed to represent a name, normally used to validate container and volume names.
5
+const RestrictedNameChars = `[a-zA-Z0-9][a-zA-Z0-9_.-]`
6
+
7
+// RestrictedNamePattern is a regular expression to validate names against the collection of restricted characters.
8
+var RestrictedNamePattern = regexp.MustCompile(`^/?` + RestrictedNameChars + `+$`)
... ...
@@ -11,7 +11,9 @@ import (
11 11
 	"path/filepath"
12 12
 	"sync"
13 13
 
14
+	derr "github.com/docker/docker/errors"
14 15
 	"github.com/docker/docker/pkg/idtools"
16
+	"github.com/docker/docker/utils"
15 17
 	"github.com/docker/docker/volume"
16 18
 )
17 19
 
... ...
@@ -23,8 +25,14 @@ const (
23 23
 	volumesPathName    = "volumes"
24 24
 )
25 25
 
26
-// ErrNotFound is the typed error returned when the requested volume name can't be found
27
-var ErrNotFound = errors.New("volume not found")
26
+var (
27
+	// ErrNotFound is the typed error returned when the requested volume name can't be found
28
+	ErrNotFound = errors.New("volume not found")
29
+	// volumeNameRegex ensures the name asigned for the volume is valid.
30
+	// This name is used to create the bind directory, so we need to avoid characters that
31
+	// would make the path to escape the root directory.
32
+	volumeNameRegex = utils.RestrictedNamePattern
33
+)
28 34
 
29 35
 // New instantiates a new Root instance with the provided scope. Scope
30 36
 // is the base path that the Root instance uses to store its
... ...
@@ -96,6 +104,10 @@ func (r *Root) Name() string {
96 96
 // the underlying directory tree required for this volume in the
97 97
 // process.
98 98
 func (r *Root) Create(name string, _ map[string]string) (volume.Volume, error) {
99
+	if err := r.validateName(name); err != nil {
100
+		return nil, err
101
+	}
102
+
99 103
 	r.m.Lock()
100 104
 	defer r.m.Unlock()
101 105
 
... ...
@@ -174,6 +186,13 @@ func (r *Root) Get(name string) (volume.Volume, error) {
174 174
 	return v, nil
175 175
 }
176 176
 
177
+func (r *Root) validateName(name string) error {
178
+	if !volumeNameRegex.MatchString(name) {
179
+		return derr.ErrorCodeVolumeName.WithArgs(name, utils.RestrictedNameChars)
180
+	}
181
+	return nil
182
+}
183
+
177 184
 // localVolume implements the Volume interface from the volume package and
178 185
 // represents the volumes created by Root.
179 186
 type localVolume struct {
... ...
@@ -79,3 +79,48 @@ func TestInitializeWithVolumes(t *testing.T) {
79 79
 		t.Fatal("expected to re-initialize root with existing volumes")
80 80
 	}
81 81
 }
82
+
83
+func TestCreate(t *testing.T) {
84
+	rootDir, err := ioutil.TempDir("", "local-volume-test")
85
+	if err != nil {
86
+		t.Fatal(err)
87
+	}
88
+	defer os.RemoveAll(rootDir)
89
+
90
+	r, err := New(rootDir, 0, 0)
91
+	if err != nil {
92
+		t.Fatal(err)
93
+	}
94
+
95
+	cases := map[string]bool{
96
+		"name":                  true,
97
+		"name-with-dash":        true,
98
+		"name_with_underscore":  true,
99
+		"name/with/slash":       false,
100
+		"name/with/../../slash": false,
101
+		"./name":                false,
102
+		"../name":               false,
103
+		"./":                    false,
104
+		"../":                   false,
105
+		"~":                     false,
106
+		".":                     false,
107
+		"..":                    false,
108
+		"...":                   false,
109
+	}
110
+
111
+	for name, success := range cases {
112
+		v, err := r.Create(name, nil)
113
+		if success {
114
+			if err != nil {
115
+				t.Fatal(err)
116
+			}
117
+			if v.Name() != name {
118
+				t.Fatalf("Expected volume with name %s, got %s", name, v.Name())
119
+			}
120
+		} else {
121
+			if err == nil {
122
+				t.Fatalf("Expected error creating volume with name %s, got nil", name)
123
+			}
124
+		}
125
+	}
126
+}