fdb permissions config fix

Allow only one of UID, GID to be specified. Previously either both or
neither had to be specified.

©! I hereby licence these changes under the licence with SHA256 hash
©! fd80a26fbb3f644af1fa994134446702932968519797227e07a1368dea80f0bc.

fdb/fdb.go

@@ -191,30 +191,31 @@ func (db *DB) loadPermissions() error {
 	return nil
 }
 
-func resolveUIDGID(p *Permission) (uid, gid int, enforce bool, err error) {
-	if p.UID == "" || p.GID == "" {
-		return
-	}
-
+// Returns the UID and GID to enforce. If the UID or GID is -1, it is not
+// to be enforced. Neither or both or either of the UID or GID may be -1.
+func resolveUIDGID(p *Permission) (uid, gid int, err error) {
 	if p.UID == "$r" {
 		uid = os.Getuid()
-	} else {
+	} else if p.UID != "" {
 		uid, err = passwd.ParseUID(p.UID)
 		if err != nil {
 			return
 		}
+	} else {
+		uid = -1
 	}
 
 	if p.GID == "$r" {
 		gid = os.Getgid()
-	} else {
+	} else if p.GID != "" {
 		gid, err = passwd.ParseGID(p.GID)
 		if err != nil {
 			return
 		}
+	} else {
+		gid = -1
 	}
 
-	enforce = true
 	return
 }
 
@@ -295,12 +296,12 @@ func (db *DB) conformPermissions() error {
 				}
 			}
 
-			correctUID, correctGID, enforceOwner, err := resolveUIDGID(perm)
+			correctUID, correctGID, err := resolveUIDGID(perm)
 			if err != nil {
 				return err
 			}
 
-			if enforceOwner {
+			if correctUID >= 0 || correctGID >= 0 {
 				curUID, err := deos.GetFileUID(info)
 				if err != nil {
 					return err
@@ -311,6 +312,14 @@ func (db *DB) conformPermissions() error {
 					return err
 				}
 
+				if correctUID < 0 {
+					correctUID = curUID
+				}
+
+				if correctGID < 0 {
+					correctGID = curGID
+				}
+
 				if curUID != correctUID || curGID != correctGID {
 					log.Warnf("%#v has wrong UID/GID %v/%v, changing to %v/%v", rpath, curUID, curGID, correctUID, correctGID)
 
@@ -610,17 +619,24 @@ func (cw *closeWrapper) Close() error {
 		if p != nil {
 			// TempFile creates files with mode 0600, so it's OK to chmod/chown it here, race-wise
 
-			if p.UID != "" {
-				uid, gid, enforce, err := resolveUIDGID(p)
-				if err != nil {
-					return err
-				}
+			correctUID, correctGID, err := resolveUIDGID(p)
+			if err != nil {
+				return err
+			}
 
-				if enforce && (uid != os.Getuid() || gid != os.Getgid()) {
-					err := os.Lchown(cw.finalName, uid, gid)
-					// failure is nonfatal, may not be root
-					log.Errore(err, "could not set correct owner for file", cw.finalName)
-				}
+			curUID := os.Getuid()
+			curGID := os.Getgid()
+			if correctUID < 0 {
+				correctUID = curUID
+			}
+			if correctGID < 0 {
+				correctGID = curGID
+			}
+
+			if correctUID != curUID || correctGID != curGID {
+				err := os.Lchown(cw.finalName, correctUID, correctGID)
+				// failure is nonfatal, may not be root
+				log.Errore(err, "could not set correct owner for file", cw.finalName)
 			}
 
 			err = os.Chmod(cw.finalName, p.FileMode)

fdb/parseperm_test.go

@@ -22,12 +22,16 @@ func TestParsePerm(t *testing.T) {
      beta  0644 0755  42 42
      gamma  0644 0755  $r $r
      delta   inherit
+     x 0644 0755 root -
+     y 0644 0755 - root
      `, []Permission{
 			{Path: "foo/bar", FileMode: 0644, DirMode: 0755},
 			{Path: "foo/*/baz", FileMode: 0640, DirMode: 0750},
 			{Path: "alpha", FileMode: 0644, DirMode: 0755, UID: "root", GID: "root"},
 			{Path: "beta", FileMode: 0644, DirMode: 0755, UID: "42", GID: "42"},
 			{Path: "gamma", FileMode: 0644, DirMode: 0755, UID: "$r", GID: "$r"},
+			{Path: "x", FileMode: 0644, DirMode: 0755, UID: "root", GID: ""},
+			{Path: "y", FileMode: 0644, DirMode: 0755, UID: "", GID: "root"},
 		}, map[string]struct{}{"delta": struct{}{}}},
 	}
 
@@ -38,7 +42,7 @@ func TestParsePerm(t *testing.T) {
 		}
 
 		if !reflect.DeepEqual(ps, tst.Out) {
-			t.Fatalf("permissions don't match: got %v, expected %v", ps, tst.Out)
+			t.Fatalf("permissions don't match: got %#v, expected %#v", ps, tst.Out)
 		}
 
 		if !reflect.DeepEqual(erase, tst.Erase) {