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 special case of auth context LeaseRevokeExpire. It
indicates that LeaseRevoke was issued internally so it should be able
to delete any attached keys. In such a case, the permission check can
be omitted.
  • Loading branch information
mitake committed Jun 7, 2017
1 parent 3cbbb54 commit f6ba5d1
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 4 deletions.
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
37 changes: 34 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,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 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 +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:
Expand Down
3 changes: 2 additions & 1 deletion etcdserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,8 @@ func (s *EtcdServer) run() {
}
lid := lease.ID
s.goAttach(func() {
s.LeaseRevoke(s.ctx, &pb.LeaseRevokeRequest{ID: int64(lid)})
ctx := context.WithValue(s.ctx, "LeaseRevokeExpire", struct{}{})
s.LeaseRevoke(ctx, &pb.LeaseRevokeRequest{ID: int64(lid)})
<-c
})
}
Expand Down
11 changes: 11 additions & 0 deletions etcdserver/v3_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -744,5 +744,16 @@ func (s *EtcdServer) AuthInfoFromCtx(ctx context.Context) (*auth.AuthInfo, error
}
}

if ctx.Value("LeaseRevokeExpire") != nil {
// LeaseRevoke issued by time progress is generated internally so
// it doesn't have a user that issued the request directly.
// For deleting every key that the lease is attached, the request
// needs root priviledge.
return &auth.AuthInfo{
Username: "root",
Revision: s.AuthStore().Revision(),
}, nil
}

return s.AuthStore().AuthInfoFromCtx(ctx)
}

0 comments on commit f6ba5d1

Please sign in to comment.