Merge pull request #15814 from mitake/backport-15656-3.4

Backport 15656 to release-3.4
etcd-io · May 10, 2023 · 3f78c42 · 3f78c42
2 parents 2db96e8 + c62b5db
commit 3f78c42
Show file tree

Hide file tree

Showing 4 changed files with 193 additions and 6 deletions.
diff --git a/etcdserver/v3_server.go b/etcdserver/v3_server.go
@@ -322,7 +322,32 @@ func (s *EtcdServer) LeaseRenew(ctx context.Context, id lease.LeaseID) (int64, e
 	return -1, ErrCanceled
 }
 
-func (s *EtcdServer) LeaseTimeToLive(ctx context.Context, r *pb.LeaseTimeToLiveRequest) (*pb.LeaseTimeToLiveResponse, error) {
+func (s *EtcdServer) checkLeaseTimeToLive(ctx context.Context, leaseID lease.LeaseID) (uint64, error) {
+	rev := s.AuthStore().Revision()
+	if !s.AuthStore().IsAuthEnabled() {
+		return rev, nil
+	}
+	authInfo, err := s.AuthInfoFromCtx(ctx)
+	if err != nil {
+		return rev, err
+	}
+	if authInfo == nil {
+		return rev, auth.ErrUserEmpty
+	}
+
+	l := s.lessor.Lookup(leaseID)
+	if l != nil {
+		for _, key := range l.Keys() {
+			if err := s.AuthStore().IsRangePermitted(authInfo, []byte(key), []byte{}); err != nil {
+				return 0, err
+			}
+		}
+	}
+
+	return rev, nil
+}
+
+func (s *EtcdServer) leaseTimeToLive(ctx context.Context, r *pb.LeaseTimeToLiveRequest) (*pb.LeaseTimeToLiveResponse, error) {
 	if s.isLeader() {
 		if err := s.waitAppliedIndex(); err != nil {
 			return nil, err
@@ -372,6 +397,31 @@ func (s *EtcdServer) LeaseTimeToLive(ctx context.Context, r *pb.LeaseTimeToLiveR
 	return nil, ErrCanceled
 }
 
+func (s *EtcdServer) LeaseTimeToLive(ctx context.Context, r *pb.LeaseTimeToLiveRequest) (*pb.LeaseTimeToLiveResponse, error) {
+	var rev uint64
+	var err error
+	if r.Keys {
+		// check RBAC permission only if Keys is true
+		rev, err = s.checkLeaseTimeToLive(ctx, lease.LeaseID(r.ID))
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	resp, err := s.leaseTimeToLive(ctx, r)
+	if err != nil {
+		return nil, err
+	}
+
+	if r.Keys {
+		if s.AuthStore().IsAuthEnabled() && rev != s.AuthStore().Revision() {
+			return nil, auth.ErrAuthOldRevision
+		}
+	}
+	return resp, nil
+}
+
+// LeaseLeases is really ListLeases !???
 func (s *EtcdServer) LeaseLeases(ctx context.Context, r *pb.LeaseLeasesRequest) (*pb.LeaseLeasesResponse, error) {
 	ls := s.lessor.Leases()
 	lss := make([]*pb.LeaseStatus, len(ls))

diff --git a/integration/v3_auth_test.go b/integration/v3_auth_test.go
@@ -150,12 +150,10 @@ func testV3AuthWithLeaseRevokeWithRoot(t *testing.T, ccfg ClusterConfig) {
 	// wait for lease expire
 	time.Sleep(3 * time.Second)
 
-	tresp, terr := api.Lease.LeaseTimeToLive(
+	tresp, terr := rootc.TimeToLive(
 		context.TODO(),
-		&pb.LeaseTimeToLiveRequest{
-			ID:   int64(leaseID),
-			Keys: true,
-		},
+		leaseID,
+		clientv3.WithAttachedKeys(),
 	)
 	if terr != nil {
 		t.Error(terr)
@@ -526,3 +524,85 @@ func TestV3AuthWatchErrorAndWatchId0(t *testing.T) {
 
 	<-watchEndCh
 }
+
+func TestV3AuthWithLeaseTimeToLive(t *testing.T) {
+	clus := NewClusterV3(t, &ClusterConfig{Size: 1})
+	defer clus.Terminate(t)
+
+	users := []user{
+		{
+			name:     "user1",
+			password: "user1-123",
+			role:     "role1",
+			key:      "k1",
+			end:      "k3",
+		},
+		{
+			name:     "user2",
+			password: "user2-123",
+			role:     "role2",
+			key:      "k2",
+			end:      "k4",
+		},
+	}
+	authSetupUsers(t, toGRPC(clus.Client(0)).Auth, users)
+
+	authSetupRoot(t, toGRPC(clus.Client(0)).Auth)
+
+	user1c, cerr := clientv3.New(clientv3.Config{Endpoints: clus.Client(0).Endpoints(), Username: "user1", Password: "user1-123"})
+	if cerr != nil {
+		t.Fatal(cerr)
+	}
+	defer user1c.Close()
+
+	user2c, cerr := clientv3.New(clientv3.Config{Endpoints: clus.Client(0).Endpoints(), Username: "user2", Password: "user2-123"})
+	if cerr != nil {
+		t.Fatal(cerr)
+	}
+	defer user2c.Close()
+
+	leaseResp, err := user1c.Grant(context.TODO(), 90)
+	if err != nil {
+		t.Fatal(err)
+	}
+	leaseID := leaseResp.ID
+	_, err = user1c.Put(context.TODO(), "k1", "val", clientv3.WithLease(leaseID))
+	if err != nil {
+		t.Fatal(err)
+	}
+	// k2 can be accessed from both user1 and user2
+	_, err = user1c.Put(context.TODO(), "k2", "val", clientv3.WithLease(leaseID))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = user1c.TimeToLive(context.TODO(), leaseID)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = user2c.TimeToLive(context.TODO(), leaseID)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = user2c.TimeToLive(context.TODO(), leaseID, clientv3.WithAttachedKeys())
+	if err == nil {
+		t.Fatal("timetolive from user2 should be failed with permission denied")
+	}
+
+	rootc, cerr := clientv3.New(clientv3.Config{Endpoints: clus.Client(0).Endpoints(), Username: "root", Password: "123"})
+	if cerr != nil {
+		t.Fatal(cerr)
+	}
+	defer rootc.Close()
+
+	if _, err := rootc.RoleRevokePermission(context.TODO(), "role1", "k1", "k3"); err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = user1c.TimeToLive(context.TODO(), leaseID, clientv3.WithAttachedKeys())
+	if err == nil {
+		t.Fatal("timetolive from user2 should be failed with permission denied")
+	}
+}
diff --git a/tests/e2e/ctl_v3_auth_test.go b/tests/e2e/ctl_v3_auth_test.go
@@ -68,6 +68,7 @@ func TestCtlV3AuthSnapshot(t *testing.T)        { testCtl(t, authTestSnapshot) }
 func TestCtlV3AuthSnapshotJWT(t *testing.T)     { testCtl(t, authTestSnapshot, withCfg(configJWT)) }
 func TestCtlV3AuthJWTExpire(t *testing.T)       { testCtl(t, authTestJWTExpire, withCfg(configJWT)) }
 func TestCtlV3AuthTestCacheReload(t *testing.T) { testCtl(t, authTestCacheReload) }
+func TestCtlV3AuthLeaseTimeToLive(t *testing.T) { testCtl(t, authTestLeaseTimeToLive) }
 
 func authEnableTest(cx ctlCtx) {
 	if err := authEnable(cx); err != nil {
@@ -1233,3 +1234,51 @@ func authTestCacheReload(cx ctlCtx) {
 		cx.t.Fatal(err)
 	}
 }
+
+func authTestLeaseTimeToLive(cx ctlCtx) {
+	if err := authEnable(cx); err != nil {
+		cx.t.Fatal(err)
+	}
+	cx.user, cx.pass = "root", "root"
+
+	authSetupTestUser(cx)
+
+	cx.user = "test-user"
+	cx.pass = "pass"
+
+	leaseID, err := ctlV3LeaseGrant(cx, 10)
+	if err != nil {
+		cx.t.Fatal(err)
+	}
+
+	err = ctlV3Put(cx, "foo", "val", leaseID)
+	if err != nil {
+		cx.t.Fatal(err)
+	}
+
+	err = ctlV3LeaseTimeToLive(cx, leaseID, true)
+	if err != nil {
+		cx.t.Fatal(err)
+	}
+
+	cx.user = "root"
+	cx.pass = "root"
+	err = ctlV3Put(cx, "bar", "val", leaseID)
+	if err != nil {
+		cx.t.Fatal(err)
+	}
+
+	cx.user = "test-user"
+	cx.pass = "pass"
+	// the lease is attached to bar, which test-user cannot access
+	err = ctlV3LeaseTimeToLive(cx, leaseID, true)
+	if err == nil {
+		cx.t.Fatal("test-user must not be able to access to the lease, because it's attached to the key bar")
+	}
+
+	// without --keys, access should be allowed
+	err = ctlV3LeaseTimeToLive(cx, leaseID, false)
+	if err != nil {
+		cx.t.Fatal(err)
+	}
+}
diff --git a/tests/e2e/ctl_v3_lease_test.go b/tests/e2e/ctl_v3_lease_test.go
@@ -294,3 +294,11 @@ func ctlV3LeaseRevoke(cx ctlCtx, leaseID string) error {
 	cmdArgs := append(cx.PrefixArgs(), "lease", "revoke", leaseID)
 	return spawnWithExpect(cmdArgs, fmt.Sprintf("lease %s revoked", leaseID))
 }
+
+func ctlV3LeaseTimeToLive(cx ctlCtx, leaseID string, withKeys bool) error {
+	cmdArgs := append(cx.PrefixArgs(), "lease", "timetolive", leaseID)
+	if withKeys {
+		cmdArgs = append(cmdArgs, "--keys")
+	}
+	return spawnWithExpect(cmdArgs, fmt.Sprintf("lease %s granted with", leaseID))
+}