Merge pull request coreos#5 from unihorn/30

refactor(depot): use tag instead of name

depot/depot.go

@@ -7,17 +7,33 @@ import (
 	"path/filepath"
 )
 
+// Tag includes name and permission requirement
+// Permission requirement is used in two ways:
+// 1. Set the permission for data when Put
+// 2. Check the permission required when Get
+// It is set to prevent attacks from other users for FileDepot.
+// For example, 'evil' creates file ca.key with 0666 file perm,
+// 'core' reads it and uses it as ca.key. It may cause the security
+// problem of fake certificate and key.
+type Tag struct {
+	name string
+	// TODO(yichengq): make perm module take in charge later
+	perm os.FileMode
+}
+
 // Depot is in charge of data storage
 type Depot interface {
-	Put(name string, data []byte, perm os.FileMode) error
-	Get(name string) ([]byte, error)
-	Delete(name string)
+	Put(tag *Tag, data []byte) error
+	Get(tag *Tag) ([]byte, error)
+	Delete(tag *Tag) error
+	Check(tag *Tag) bool
 }
 
+
 // FileDepot is a implementation of Depot using file system
 type FileDepot struct {
 	// Absolute path of directory that holds all files
-	Dir string
+	dirPath string
 }
 
 func New(dir string) (*FileDepot, error) {
@@ -32,37 +48,51 @@ func New(dir string) (*FileDepot, error) {
 }
 
 func (d *FileDepot) path(name string) string {
-	return filepath.Join(d.Dir, name)
+	return filepath.Join(d.dirPath, name)
 }
 
-func (d *FileDepot) Put(name string, data []byte, perm os.FileMode) error {
+func (d *FileDepot) Put(tag *Tag, data []byte) error {
 	if data == nil {
 		return errors.New("data is nil")
 	}
 
-	if err := os.MkdirAll(d.Dir, 0755); err != nil {
+	if err := os.MkdirAll(d.dirPath, 0755); err != nil {
 		return err
 	}
 
-	file, err := os.OpenFile(d.path(name), os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm)
+	name := d.path(tag.name)
+	perm := tag.perm
+
+	file, err := os.OpenFile(name, os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm)
 	if err != nil {
 		return err
 	}
 
 	if _, err := file.Write(data); err != nil {
 		file.Close()
-		os.Remove(d.path(name))
+		os.Remove(name)
 		return err
 	}
 
 	file.Close()
 	return nil
 }
 
-func (d *FileDepot) Get(name string) ([]byte, error) {
-	return ioutil.ReadFile(d.path(name))
+func (d *FileDepot) Check(tag *Tag) bool {
+	name := d.path(tag.name)
+	if fi, err := os.Stat(name); err == nil && ^fi.Mode() & tag.perm == 0 {
+		return true
+	}
+	return false
+}
+
+func (d *FileDepot) Get(tag *Tag) ([]byte, error) {
+	if !d.Check(tag) {
+		return nil, errors.New("permission denied")
+	}
+	return ioutil.ReadFile(d.path(tag.name))
 }
 
-func (d *FileDepot) Delete(name string) {
-	os.Remove(d.path(name))
+func (d *FileDepot) Delete(tag *Tag) error {
+	return os.Remove(d.path(tag.name))
 }

depot/depot_test.go

@@ -7,11 +7,16 @@ import (
 )
 
 const (
-	name = "host.pem"
 	data = "It is a trap only!"
 	dir = ".etcd-ca"
 )
 
+var (
+	tag = &Tag{"host.pem", 0600}
+	wrongTag = &Tag{"host.pem", 0666}
+	wrongTag2 = &Tag{"host.pem2", 0600}
+)
+
 func getDepot(t *testing.T) Depot {
 	os.RemoveAll(dir)
 
@@ -26,40 +31,75 @@ func getDepot(t *testing.T) Depot {
 func TestDepotCRUD(t *testing.T) {
 	d := getDepot(t)
 
-	if err := d.Put(name, []byte(data), 0666); err != nil {
+	if err := d.Put(tag, []byte(data)); err != nil {
 		t.Fatal("Failed putting file into Depot:", err)
 	}
 
-	dataRead, err := d.Get(name)
+	dataRead, err := d.Get(tag)
 	if err != nil {
 		t.Fatal("Failed getting file from Depot:", err)
 	}
-
 	if bytes.Compare(dataRead, []byte(data)) != 0 {
 		t.Fatal("Failed getting the previous data")
 	}
 
-	if err = d.Put(name, []byte(data), 0666); err == nil || !os.IsExist(err) {
+	if err = d.Put(tag, []byte(data)); err == nil || !os.IsExist(err) {
 		t.Fatal("Expect not to put file into Depot:", err)
 	}
 
-	d.Delete(name)
+	d.Delete(tag)
 
-	if _, err = d.Get(name); err == nil || !os.IsNotExist(err) {
+	if d.Check(tag) {
 		t.Fatal("Failed deleting file from Depot:", err)
 	}
 }
 
 func TestDepotPutNil(t *testing.T) {
 	d := getDepot(t)
 
-	if err := d.Put(name, nil, 0666); err == nil {
+	if err := d.Put(tag, nil); err == nil {
 		t.Fatal("Expect not to put nil into Depot:", err)
 	}
 
-	if err := d.Put(name, []byte(data), 0666); err != nil {
+	if err := d.Put(tag, []byte(data)); err != nil {
 		t.Fatal("Failed putting file into Depot:", err)
 	}
 
-	d.Delete(name)
+	d.Delete(tag)
+}
+
+func TestDepotCheckFailure(t *testing.T) {
+	d := getDepot(t)
+
+	if err := d.Put(tag, []byte(data)); err != nil {
+		t.Fatal("Failed putting file into Depot:", err)
+	}
+
+	if d.Check(wrongTag) {
+		t.Fatal("Expect not to checking out file with insufficient permission")
+	}
+
+	if d.Check(wrongTag2) {
+		t.Fatal("Expect not to checking out file with nonexist name")
+	}
+
+	d.Delete(tag)
+}
+
+func TestDepotGetFailure(t *testing.T) {
+	d := getDepot(t)
+
+	if err := d.Put(tag, []byte(data)); err != nil {
+		t.Fatal("Failed putting file into Depot:", err)
+	}
+
+	if _, err := d.Get(wrongTag); err == nil {
+		t.Fatal("Expect not to checking out file with insufficient permission")
+	}
+
+	if _, err := d.Get(wrongTag2); err == nil {
+		t.Fatal("Expect not to checking out file with nonexist name")
+	}
+
+	d.Delete(tag)
 }