Merge pull request #8514 from mitake/empty-key-perm

etcdctl: handle empty key permission correctly
etcd-io · Sep 13, 2017 · 1b85dad · 1b85dad
2 parents 510d884 + e4c0e11
commit 1b85dad
Show file tree

Hide file tree

Showing 2 changed files with 77 additions and 11 deletions.
diff --git a/e2e/ctl_v3_auth_test.go b/e2e/ctl_v3_auth_test.go
@@ -32,7 +32,7 @@ func TestCtlV3AuthRoleUpdate(t *testing.T)          { testCtl(t, authRoleUpdateT
 func TestCtlV3AuthUserDeleteDuringOps(t *testing.T) { testCtl(t, authUserDeleteDuringOpsTest) }
 func TestCtlV3AuthRoleRevokeDuringOps(t *testing.T) { testCtl(t, authRoleRevokeDuringOpsTest) }
 func TestCtlV3AuthTxn(t *testing.T)                 { testCtl(t, authTestTxn) }
-func TestCtlV3AuthPerfixPerm(t *testing.T)          { testCtl(t, authTestPrefixPerm) }
+func TestCtlV3AuthPrefixPerm(t *testing.T)          { testCtl(t, authTestPrefixPerm) }
 func TestCtlV3AuthMemberAdd(t *testing.T)           { testCtl(t, authTestMemberAdd) }
 func TestCtlV3AuthMemberRemove(t *testing.T) {
 	testCtl(t, authTestMemberRemove, withQuorum(), withNoStrictReconfig())
@@ -469,6 +469,21 @@ func authTestPrefixPerm(cx ctlCtx) {
 	if err := ctlV3PutFailPerm(cx, clientv3.GetPrefixRangeEnd(prefix), "baz"); err != nil {
 		cx.t.Fatal(err)
 	}
+
+	// grant the entire keys to test-user
+	cx.user, cx.pass = "root", "root"
+	if err := ctlV3RoleGrantPermission(cx, "test-role", grantingPerm{true, true, "", "", true}); err != nil {
+		cx.t.Fatal(err)
+	}
+
+	prefix2 := "/prefix2/"
+	cx.user, cx.pass = "test-user", "pass"
+	for i := 0; i < 10; i++ {
+		key := fmt.Sprintf("%s%d", prefix2, i)
+		if err := ctlV3Put(cx, key, "val", ""); err != nil {
+			cx.t.Fatal(err)
+		}
+	}
 }
 
 func authTestMemberAdd(cx ctlCtx) {
@@ -675,6 +690,36 @@ func authTestFromKeyPerm(cx ctlCtx) {
 			cx.t.Fatal(err)
 		}
 	}
+
+	// grant the entire keys
+	cx.user, cx.pass = "root", "root"
+	if err := ctlV3RoleGrantPermission(cx, "test-role", grantingPerm{true, true, "", "\x00", false}); err != nil {
+		cx.t.Fatal(err)
+	}
+
+	// try keys, of course it must be allowed because test-role has a permission of the entire keys
+	cx.user, cx.pass = "test-user", "pass"
+	for i := 0; i < 10; i++ {
+		key := fmt.Sprintf("z%d", i)
+		if err := ctlV3Put(cx, key, "val", ""); err != nil {
+			cx.t.Fatal(err)
+		}
+	}
+
+	// revoke the entire keys
+	cx.user, cx.pass = "root", "root"
+	if err := ctlV3RoleRevokePermission(cx, "test-role", "", "", true); err != nil {
+		cx.t.Fatal(err)
+	}
+
+	// try the revoked entire key permission
+	cx.user, cx.pass = "test-user", "pass"
+	for i := 0; i < 10; i++ {
+		key := fmt.Sprintf("z%d", i)
+		if err := ctlV3PutFailPerm(cx, key, "val"); err != nil {
+			cx.t.Fatal(err)
+		}
+	}
 }
 
 func authTestWatch(cx ctlCtx) {

diff --git a/etcdctl/ctlv3/command/role_command.go b/etcdctl/ctlv3/command/role_command.go
@@ -170,11 +170,8 @@ func roleGrantPermissionCommandFunc(cmd *cobra.Command, args []string) {
 		ExitWithError(ExitBadArgs, err)
 	}
 
-	rangeEnd, rerr := rangeEndFromPermFlags(args[2:])
-	if rerr != nil {
-		ExitWithError(ExitBadArgs, rerr)
-	}
-	resp, err := mustClientFromCmd(cmd).Auth.RoleGrantPermission(context.TODO(), args[0], args[2], rangeEnd, perm)
+	key, rangeEnd := permRange(args[2:])
+	resp, err := mustClientFromCmd(cmd).Auth.RoleGrantPermission(context.TODO(), args[0], key, rangeEnd, perm)
 	if err != nil {
 		ExitWithError(ExitError, err)
 	}
@@ -188,17 +185,41 @@ func roleRevokePermissionCommandFunc(cmd *cobra.Command, args []string) {
 		ExitWithError(ExitBadArgs, fmt.Errorf("role revoke-permission command requires role name and key [endkey] as its argument."))
 	}
 
-	rangeEnd, rerr := rangeEndFromPermFlags(args[1:])
-	if rerr != nil {
-		ExitWithError(ExitBadArgs, rerr)
-	}
-	resp, err := mustClientFromCmd(cmd).Auth.RoleRevokePermission(context.TODO(), args[0], args[1], rangeEnd)
+	key, rangeEnd := permRange(args[1:])
+	resp, err := mustClientFromCmd(cmd).Auth.RoleRevokePermission(context.TODO(), args[0], key, rangeEnd)
 	if err != nil {
 		ExitWithError(ExitError, err)
 	}
 	display.RoleRevokePermission(args[0], args[1], rangeEnd, *resp)
 }
 
+func permRange(args []string) (string, string) {
+	key := args[0]
+	var rangeEnd string
+	if len(key) == 0 {
+		if rolePermPrefix && rolePermFromKey {
+			ExitWithError(ExitBadArgs, fmt.Errorf("--from-key and --prefix flags are mutually exclusive"))
+		}
+
+		// Range permission is expressed as adt.BytesAffineInterval,
+		// so the empty prefix which should be matched with every key must be like this ["\x00", <end>).
+		key = "\x00"
+		if rolePermPrefix || rolePermFromKey {
+			// For the both cases of prefix and from-key, a permission with an empty key
+			// should allow access to the entire key space.
+			// 0x00 will be treated as open ended in server side.
+			rangeEnd = "\x00"
+		}
+	} else {
+		var err error
+		rangeEnd, err = rangeEndFromPermFlags(args[0:])
+		if err != nil {
+			ExitWithError(ExitBadArgs, err)
+		}
+	}
+	return key, rangeEnd
+}
+
 func rangeEndFromPermFlags(args []string) (string, error) {
 	if len(args) == 1 {
 		if rolePermPrefix {