Skip to content

Commit

Permalink
etcdserver: protect revoking lease with auth
Browse files Browse the repository at this point in the history
Currently clients can revoke any lease without permission. This commit
lets etcdserver protect revoking with write permission.

This commit adds a new field Expire to LeaseRevokeRequest. The flag is
used for indicating that the revoking operation is issued by time
progress. In such a case, the permission check can be omitted.
  • Loading branch information
mitake committed Jun 6, 2017
1 parent 3cbbb54 commit 767c0b3
Show file tree
Hide file tree
Showing 6 changed files with 198 additions and 120 deletions.
5 changes: 5 additions & 0 deletions Documentation/dev-guide/apispec/swagger/rpc.swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -1607,6 +1607,11 @@
"type": "string",
"format": "int64",
"description": "ID is the lease ID to revoke. When the ID is revoked, all associated keys will be deleted."
},
"expire": {
"type": "boolean",
"format": "boolean",
"description": "expire is the flag for indicating that the revoke request is invoked by expire.\nIn such a case, permission checking can be skipped."
}
}
},
Expand Down
1 change: 1 addition & 0 deletions etcdserver/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ func (s *EtcdServer) newApplierV3() applierV3 {
return newAuthApplierV3(
s.AuthStore(),
newQuotaApplierV3(s, &applierV3backend{s}),
s.lessor,
)
}

Expand Down
38 changes: 35 additions & 3 deletions etcdserver/apply_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -63,6 +65,20 @@ 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
}

lease := aa.lessor.Lookup(lease.LeaseID(r.Lease))
if lease != nil {
for _, key := range lease.Keys() {
if err := aa.as.IsPutPermitted(&aa.authInfo, []byte(key)); err != nil {
// The especified 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 {
Expand Down Expand Up @@ -158,6 +174,22 @@ 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 !lc.Expire {
// If lc.Expire == true, it means LeaseRevoke() is invoked by time progress,
// no permission check is required.
lease := aa.lessor.Lookup(lease.LeaseID(lc.ID))
if lease != nil {
for _, key := range lease.Keys() {
if err := aa.as.IsPutPermitted(&aa.authInfo, []byte(key)); err != nil {
return nil, err
}
}
}
}
return aa.applierV3.LeaseRevoke(lc)
}

func needAdminPermission(r *pb.InternalRaftRequest) bool {
switch {
case r.AuthEnable != nil:
Expand Down
Loading

0 comments on commit 767c0b3

Please sign in to comment.