Merge pull request #8031 from mitake/lease-revoke-auth

protecting lease revoking with auth

Author: mitake

auth/simple_token.go

@@ -118,14 +118,15 @@ func (t *tokenSimple) genTokenPrefix() (string, error) {
 
 func (t *tokenSimple) assignSimpleTokenToUser(username, token string) {
 	t.simpleTokensMu.Lock()
+	defer t.simpleTokensMu.Unlock()
+
 	_, ok := t.simpleTokens[token]
 	if ok {
 		plog.Panicf("token %s is alredy used", token)
 	}
 
 	t.simpleTokens[token] = username
 	t.simpleTokenKeeper.addSimpleToken(token)
-	t.simpleTokensMu.Unlock()
 }
 
 func (t *tokenSimple) invalidateUser(username string) {

auth/store.go

@@ -162,6 +162,9 @@ type AuthStore interface {
 
 	// AuthInfoFromTLS gets AuthInfo from TLS info of gRPC's context
 	AuthInfoFromTLS(ctx context.Context) *AuthInfo
+
+	// WithRoot generates and installs a token that can be used as a root credential
+	WithRoot(ctx context.Context) context.Context
 }
 
 type TokenProvider interface {
@@ -1057,3 +1060,35 @@ func NewTokenProvider(tokenOpts string, indexWaiter func(uint64) <-chan struct{}
 		return nil, ErrInvalidAuthOpts
 	}
 }
+
+func (as *authStore) WithRoot(ctx context.Context) context.Context {
+	if !as.isAuthEnabled() {
+		return ctx
+	}
+
+	var ctxForAssign context.Context
+	if ts := as.tokenProvider.(*tokenSimple); ts != nil {
+		ctx1 := context.WithValue(ctx, "index", uint64(0))
+		prefix, err := ts.genTokenPrefix()
+		if err != nil {
+			plog.Errorf("failed to generate prefix of internally used token")
+			return ctx
+		}
+		ctxForAssign = context.WithValue(ctx1, "simpleToken", prefix)
+	} else {
+		ctxForAssign = ctx
+	}
+
+	token, err := as.tokenProvider.assign(ctxForAssign, "root", as.Revision())
+	if err != nil {
+		// this must not happen
+		plog.Errorf("failed to assign token for lease revoking: %s", err)
+		return ctx
+	}
+
+	mdMap := map[string]string{
+		"token": token,
+	}
+	tokenMD := metadata.New(mdMap)
+	return metadata.NewContext(ctx, tokenMD)
+}

etcdserver/apply.go

@@ -84,6 +84,7 @@ func (s *EtcdServer) newApplierV3() applierV3 {
 	return newAuthApplierV3(
 		s.AuthStore(),
 		newQuotaApplierV3(s, &applierV3backend{s}),
+		s.lessor,
 	)
 }
 

etcdserver/apply_auth.go

@@ -19,12 +19,14 @@ import (
 
 	"github.com/coreos/etcd/auth"
 	pb "github.com/coreos/etcd/etcdserver/etcdserverpb"
+	"github.com/coreos/etcd/lease"
 	"github.com/coreos/etcd/mvcc"
 )
 
 type authApplierV3 struct {
 	applierV3
-	as auth.AuthStore
+	as     auth.AuthStore
+	lessor lease.Lessor
 
 	// mu serializes Apply so that user isn't corrupted and so that
 	// serialized requests don't leak data from TOCTOU errors
@@ -33,8 +35,8 @@ type authApplierV3 struct {
 	authInfo auth.AuthInfo
 }
 
-func newAuthApplierV3(as auth.AuthStore, base applierV3) *authApplierV3 {
-	return &authApplierV3{applierV3: base, as: as}
+func newAuthApplierV3(as auth.AuthStore, base applierV3, lessor lease.Lessor) *authApplierV3 {
+	return &authApplierV3{applierV3: base, as: as, lessor: lessor}
 }
 
 func (aa *authApplierV3) Apply(r *pb.InternalRaftRequest) *applyResult {
@@ -63,6 +65,15 @@ func (aa *authApplierV3) Put(txn mvcc.TxnWrite, r *pb.PutRequest) (*pb.PutRespon
 	if err := aa.as.IsPutPermitted(&aa.authInfo, r.Key); err != nil {
 		return nil, err
 	}
+
+	if err := aa.checkLeasePuts(lease.LeaseID(r.Lease)); err != nil {
+		// The specified lease is already attached with a key that cannot
+		// be written by this user. It means the user cannot revoke the
+		// lease so attaching the lease to the newly written key should
+		// be forbidden.
+		return nil, err
+	}
+
 	if r.PrevKv {
 		err := aa.as.IsRangePermitted(&aa.authInfo, r.Key, nil)
 		if err != nil {
@@ -158,6 +169,26 @@ func (aa *authApplierV3) Txn(rt *pb.TxnRequest) (*pb.TxnResponse, error) {
 	return aa.applierV3.Txn(rt)
 }
 
+func (aa *authApplierV3) LeaseRevoke(lc *pb.LeaseRevokeRequest) (*pb.LeaseRevokeResponse, error) {
+	if err := aa.checkLeasePuts(lease.LeaseID(lc.ID)); err != nil {
+		return nil, err
+	}
+	return aa.applierV3.LeaseRevoke(lc)
+}
+
+func (aa *authApplierV3) checkLeasePuts(leaseID lease.LeaseID) error {
+	lease := aa.lessor.Lookup(leaseID)
+	if lease != nil {
+		for _, key := range lease.Keys() {
+			if err := aa.as.IsPutPermitted(&aa.authInfo, []byte(key)); err != nil {
+				return err
+			}
+		}
+	}
+
+	return nil
+}
+
 func needAdminPermission(r *pb.InternalRaftRequest) bool {
 	switch {
 	case r.AuthEnable != nil:

etcdserver/server.go

@@ -747,7 +747,8 @@ func (s *EtcdServer) run() {
 					}
 					lid := lease.ID
 					s.goAttach(func() {
-						s.LeaseRevoke(s.ctx, &pb.LeaseRevokeRequest{ID: int64(lid)})
+						ctx := s.authStore.WithRoot(s.ctx)
+						s.LeaseRevoke(ctx, &pb.LeaseRevokeRequest{ID: int64(lid)})
 						<-c
 					})
 				}

integration/v3_auth_test.go

@@ -20,6 +20,7 @@ import (
 
 	"golang.org/x/net/context"
 
+	"github.com/coreos/etcd/auth/authpb"
 	"github.com/coreos/etcd/clientv3"
 	"github.com/coreos/etcd/etcdserver/api/v3rpc/rpctypes"
 	pb "github.com/coreos/etcd/etcdserver/etcdserverpb"
@@ -104,16 +105,167 @@ func TestV3AuthRevision(t *testing.T) {
 	}
 }
 
-func authSetupRoot(t *testing.T, auth pb.AuthClient) {
-	if _, err := auth.UserAdd(context.TODO(), &pb.AuthUserAddRequest{Name: "root", Password: "123"}); err != nil {
+type user struct {
+	name     string
+	password string
+	role     string
+	key      string
+	end      string
+}
+
+func TestV3AuthWithLeaseRevoke(t *testing.T) {
+	defer testutil.AfterTest(t)
+	clus := NewClusterV3(t, &ClusterConfig{Size: 1})
+	defer clus.Terminate(t)
+
+	users := []user{
+		{
+			name:     "user1",
+			password: "user1-123",
+			role:     "role1",
+			key:      "k1",
+			end:      "k2",
+		},
+	}
+	authSetupUsers(t, toGRPC(clus.Client(0)).Auth, users)
+
+	authSetupRoot(t, toGRPC(clus.Client(0)).Auth)
+
+	rootc, cerr := clientv3.New(clientv3.Config{Endpoints: clus.Client(0).Endpoints(), Username: "root", Password: "123"})
+	if cerr != nil {
+		t.Fatal(cerr)
+	}
+	defer rootc.Close()
+
+	leaseResp, err := rootc.Grant(context.TODO(), 90)
+	if err != nil {
 		t.Fatal(err)
 	}
-	if _, err := auth.RoleAdd(context.TODO(), &pb.AuthRoleAddRequest{Name: "root"}); err != nil {
+	leaseID := leaseResp.ID
+	// permission of k3 isn't granted to user1
+	_, err = rootc.Put(context.TODO(), "k3", "val", clientv3.WithLease(leaseID))
+	if err != nil {
 		t.Fatal(err)
 	}
-	if _, err := auth.UserGrantRole(context.TODO(), &pb.AuthUserGrantRoleRequest{User: "root", Role: "root"}); err != nil {
+
+	userc, cerr := clientv3.New(clientv3.Config{Endpoints: clus.Client(0).Endpoints(), Username: "user1", Password: "user1-123"})
+	if cerr != nil {
+		t.Fatal(cerr)
+	}
+	defer userc.Close()
+	_, err = userc.Revoke(context.TODO(), leaseID)
+	if err == nil {
+		t.Fatal("revoking from user1 should be failed with permission denied")
+	}
+}
+
+func TestV3AuthWithLeaseAttach(t *testing.T) {
+	defer testutil.AfterTest(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
+	// permission of k2 is also granted to user2
+	_, err = user1c.Put(context.TODO(), "k2", "val", clientv3.WithLease(leaseID))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = user2c.Revoke(context.TODO(), leaseID)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	leaseResp, err = user1c.Grant(context.TODO(), 90)
+	if err != nil {
+		t.Fatal(err)
+	}
+	leaseID = leaseResp.ID
+	// permission of k1 isn't granted to user2
+	_, err = user1c.Put(context.TODO(), "k1", "val", clientv3.WithLease(leaseID))
+	if err != nil {
 		t.Fatal(err)
 	}
+
+	_, err = user2c.Revoke(context.TODO(), leaseID)
+	if err == nil {
+		t.Fatal("revoking from user2 should be failed with permission denied")
+	}
+}
+
+func authSetupUsers(t *testing.T, auth pb.AuthClient, users []user) {
+	for _, user := range users {
+		if _, err := auth.UserAdd(context.TODO(), &pb.AuthUserAddRequest{Name: user.name, Password: user.password}); err != nil {
+			t.Fatal(err)
+		}
+		if _, err := auth.RoleAdd(context.TODO(), &pb.AuthRoleAddRequest{Name: user.role}); err != nil {
+			t.Fatal(err)
+		}
+		if _, err := auth.UserGrantRole(context.TODO(), &pb.AuthUserGrantRoleRequest{User: user.name, Role: user.role}); err != nil {
+			t.Fatal(err)
+		}
+
+		if len(user.key) == 0 {
+			continue
+		}
+
+		perm := &authpb.Permission{
+			PermType: authpb.READWRITE,
+			Key:      []byte(user.key),
+			RangeEnd: []byte(user.end),
+		}
+		if _, err := auth.RoleGrantPermission(context.TODO(), &pb.AuthRoleGrantPermissionRequest{Name: user.role, Perm: perm}); err != nil {
+			t.Fatal(err)
+		}
+	}
+}
+
+func authSetupRoot(t *testing.T, auth pb.AuthClient) {
+	root := []user{
+		{
+			name:     "root",
+			password: "123",
+			role:     "root",
+			key:      "",
+		},
+	}
+	authSetupUsers(t, auth, root)
 	if _, err := auth.AuthEnable(context.TODO(), &pb.AuthEnableRequest{}); err != nil {
 		t.Fatal(err)
 	}