From d1b1aa9dbe8065fb2cb36fe035daf701ccabc4e0 Mon Sep 17 00:00:00 2001 From: Hitoshi Mitake Date: Wed, 29 Mar 2023 20:46:32 +0900 Subject: [PATCH 1/2] etcdserver: protect lease timetilive with auth Signed-off-by: Hitoshi Mitake Co-authored-by: Benjamin Wang --- server/etcdserver/v3_server.go | 52 +++++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/server/etcdserver/v3_server.go b/server/etcdserver/v3_server.go index 960a7b11f4b..9f69b86b9b1 100644 --- a/server/etcdserver/v3_server.go +++ b/server/etcdserver/v3_server.go @@ -336,7 +336,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 @@ -386,6 +411,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)) From e38eb678bbecdbe35391fa518b44c1ac14a9e322 Mon Sep 17 00:00:00 2001 From: Hitoshi Mitake Date: Fri, 31 Mar 2023 23:07:23 +0900 Subject: [PATCH 2/2] tests: e2e and integration test for timetolive Signed-off-by: Hitoshi Mitake Co-authored-by: Benjamin Wang --- tests/e2e/ctl_v3_auth_test.go | 49 +++++++++++++++++ tests/e2e/ctl_v3_lease_test.go | 8 +++ tests/integration/v3_auth_test.go | 91 +++++++++++++++++++++++++++++-- 3 files changed, 143 insertions(+), 5 deletions(-) diff --git a/tests/e2e/ctl_v3_auth_test.go b/tests/e2e/ctl_v3_auth_test.go index 0792087e062..ac7e90e0437 100644 --- a/tests/e2e/ctl_v3_auth_test.go +++ b/tests/e2e/ctl_v3_auth_test.go @@ -76,6 +76,7 @@ func TestCtlV3AuthSnapshotJWT(t *testing.T) { testCtl(t, authTestSnapsho func TestCtlV3AuthJWTExpire(t *testing.T) { testCtl(t, authTestJWTExpire, withCfg(*newConfigJWT())) } func TestCtlV3AuthRevisionConsistency(t *testing.T) { testCtl(t, authTestRevisionConsistency) } func TestCtlV3AuthTestCacheReload(t *testing.T) { testCtl(t, authTestCacheReload) } +func TestCtlV3AuthLeaseTimeToLive(t *testing.T) { testCtl(t, authTestLeaseTimeToLive) } func TestCtlV3AuthRecoverFromSnapshot(t *testing.T) { testCtl(t, authTestRecoverSnapshot, withCfg(*newConfigNoTLS()), withQuorum(), withSnapshotCount(5)) @@ -1509,3 +1510,51 @@ func hashKVs(endpoints []string, cli *clientv3.Client) ([]*clientv3.HashKVRespon } return retHashKVs, nil } + +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 index 0dc4452026f..e4e15d90547 100644 --- a/tests/e2e/ctl_v3_lease_test.go +++ b/tests/e2e/ctl_v3_lease_test.go @@ -300,3 +300,11 @@ func ctlV3LeaseRevoke(cx ctlCtx, leaseID string) error { cmdArgs := append(cx.PrefixArgs(), "lease", "revoke", leaseID) return spawnWithExpectWithEnv(cmdArgs, cx.envMap, 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 spawnWithExpectWithEnv(cmdArgs, cx.envMap, fmt.Sprintf("lease %s granted with", leaseID)) +} diff --git a/tests/integration/v3_auth_test.go b/tests/integration/v3_auth_test.go index 9a60ab02953..f030c4e164d 100644 --- a/tests/integration/v3_auth_test.go +++ b/tests/integration/v3_auth_test.go @@ -177,12 +177,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) @@ -553,3 +551,86 @@ func TestV3AuthWatchErrorAndWatchId0(t *testing.T) { <-watchEndCh } + +func TestV3AuthWithLeaseTimeToLive(t *testing.T) { + BeforeTest(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 := NewClient(t, clientv3.Config{Endpoints: clus.Client(0).Endpoints(), Username: "user1", Password: "user1-123"}) + if cerr != nil { + t.Fatal(cerr) + } + defer user1c.Close() + + user2c, cerr := NewClient(t, 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 := NewClient(t, 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") + } +}