diff --git a/CHANGELOG-3.2.md b/CHANGELOG-3.2.md index 34fe42f0c47..cb40842add7 100644 --- a/CHANGELOG-3.2.md +++ b/CHANGELOG-3.2.md @@ -13,6 +13,11 @@ See [code changes](https://github.com/coreos/etcd/compare/v3.2.16...v3.2.17) and - Fix [server panic on invalid Election Proclaim/Resign HTTP(S) requests](https://github.com/coreos/etcd/pull/9379). - Previously, wrong-formatted HTTP requests to Election API could trigger panic in etcd server. - e.g. `curl -L http://localhost:2379/v3/election/proclaim -X POST -d '{"value":""}'`, `curl -L http://localhost:2379/v3/election/resign -X POST -d '{"value":""}'`. +- Prevent [overflow by large `TTL` values for `Lease` `Grant`](https://github.com/coreos/etcd/pull/9399). + - `TTL` parameter to `Grant` request is unit of second. + - Leases with too large `TTL` values exceeding `math.MaxInt64` [expire in unexpected ways](https://github.com/coreos/etcd/issues/9374). + - Server now returns `rpctypes.ErrLeaseTTLTooLarge` to client, when the requested `TTL` is larger than 9,000,000,000 (which is >285 years). + - Again, etcd `Lease` is meant for short-periodic keepalives or sessions, in the range of seconds or minutes. Not for hours or days! - Enable etcd server [`raft.Config.CheckQuorum` when starting with `ForceNewCluster`](https://github.com/coreos/etcd/pull/9347). ### Security diff --git a/CHANGELOG-3.3.md b/CHANGELOG-3.3.md index 914c38bd2d6..454bae2e10e 100644 --- a/CHANGELOG-3.3.md +++ b/CHANGELOG-3.3.md @@ -16,6 +16,11 @@ See [code changes](https://github.com/coreos/etcd/compare/v3.3.1...v3.3.2) and [ - Fix [revision-based compaction retention parsing](https://github.com/coreos/etcd/pull/9339). - Previously, `--auto-compaction-mode revision --auto-compaction-retention 1` was [translated to revision retention 3600000000000](https://github.com/coreos/etcd/issues/9337). - Now, `--auto-compaction-mode revision --auto-compaction-retention 1` is correctly parsed as revision retention 1. +- Prevent [overflow by large `TTL` values for `Lease` `Grant`](https://github.com/coreos/etcd/pull/9399). + - `TTL` parameter to `Grant` request is unit of second. + - Leases with too large `TTL` values exceeding `math.MaxInt64` [expire in unexpected ways](https://github.com/coreos/etcd/issues/9374). + - Server now returns `rpctypes.ErrLeaseTTLTooLarge` to client, when the requested `TTL` is larger than 9,000,000,000 (which is >285 years). + - Again, etcd `Lease` is meant for short-periodic keepalives or sessions, in the range of seconds or minutes. Not for hours or days! - Enable etcd server [`raft.Config.CheckQuorum` when starting with `ForceNewCluster`](https://github.com/coreos/etcd/pull/9347). diff --git a/CHANGELOG-3.4.md b/CHANGELOG-3.4.md index 1fbc8670d25..71ce4229510 100644 --- a/CHANGELOG-3.4.md +++ b/CHANGELOG-3.4.md @@ -123,4 +123,9 @@ See [code changes](https://github.com/coreos/etcd/compare/v3.3.0...v3.4.0) and [ - Fix [revision-based compaction retention parsing](https://github.com/coreos/etcd/pull/9339). - Previously, `etcd --auto-compaction-mode revision --auto-compaction-retention 1` was [translated to revision retention 3600000000000](https://github.com/coreos/etcd/issues/9337). - Now, `etcd --auto-compaction-mode revision --auto-compaction-retention 1` is correctly parsed as revision retention 1. +- Prevent [overflow by large `TTL` values for `Lease` `Grant`](https://github.com/coreos/etcd/pull/9399). + - `TTL` parameter to `Grant` request is unit of second. + - Leases with too large `TTL` values exceeding `math.MaxInt64` [expire in unexpected ways](https://github.com/coreos/etcd/issues/9374). + - Server now returns `rpctypes.ErrLeaseTTLTooLarge` to client, when the requested `TTL` is larger than 9,000,000,000 (which is >285 years). + - Again, etcd `Lease` is meant for short-periodic keepalives or sessions, in the range of seconds or minutes. Not for hours or days! - Enable etcd server [`raft.Config.CheckQuorum` when starting with `ForceNewCluster`](https://github.com/coreos/etcd/pull/9347). diff --git a/clientv3/integration/lease_test.go b/clientv3/integration/lease_test.go index 482d258ac51..ee7611b1e40 100644 --- a/clientv3/integration/lease_test.go +++ b/clientv3/integration/lease_test.go @@ -55,6 +55,11 @@ func TestLeaseGrant(t *testing.T) { kv := clus.RandClient() + _, merr := lapi.Grant(context.Background(), clientv3.MaxLeaseTTL+1) + if merr != rpctypes.ErrLeaseTTLTooLarge { + t.Fatalf("err = %v, want %v", merr, rpctypes.ErrLeaseTTLTooLarge) + } + resp, err := lapi.Grant(context.Background(), 10) if err != nil { t.Errorf("failed to create lease %v", err) diff --git a/clientv3/grpc_options.go b/clientv3/options.go similarity index 96% rename from clientv3/grpc_options.go rename to clientv3/options.go index 592dd6993cf..fa25811f3b0 100644 --- a/clientv3/grpc_options.go +++ b/clientv3/options.go @@ -44,3 +44,6 @@ var ( // Some options are exposed to "clientv3.Config". // Defaults will be overridden by the settings in "clientv3.Config". var defaultCallOpts = []grpc.CallOption{defaultFailFast, defaultMaxCallSendMsgSize, defaultMaxCallRecvMsgSize} + +// MaxLeaseTTL is the maximum lease TTL value +const MaxLeaseTTL = 9000000000 diff --git a/etcdserver/api/v3rpc/rpctypes/error.go b/etcdserver/api/v3rpc/rpctypes/error.go index 446e4f6b870..55eab38ef17 100644 --- a/etcdserver/api/v3rpc/rpctypes/error.go +++ b/etcdserver/api/v3rpc/rpctypes/error.go @@ -31,8 +31,9 @@ var ( ErrGRPCFutureRev = status.New(codes.OutOfRange, "etcdserver: mvcc: required revision is a future revision").Err() ErrGRPCNoSpace = status.New(codes.ResourceExhausted, "etcdserver: mvcc: database space exceeded").Err() - ErrGRPCLeaseNotFound = status.New(codes.NotFound, "etcdserver: requested lease not found").Err() - ErrGRPCLeaseExist = status.New(codes.FailedPrecondition, "etcdserver: lease already exists").Err() + ErrGRPCLeaseNotFound = status.New(codes.NotFound, "etcdserver: requested lease not found").Err() + ErrGRPCLeaseExist = status.New(codes.FailedPrecondition, "etcdserver: lease already exists").Err() + ErrGRPCLeaseTTLTooLarge = status.New(codes.OutOfRange, "etcdserver: too large lease TTL").Err() ErrGRPCMemberExist = status.New(codes.FailedPrecondition, "etcdserver: member ID already exist").Err() ErrGRPCPeerURLExist = status.New(codes.FailedPrecondition, "etcdserver: Peer URLs already exists").Err() @@ -80,8 +81,9 @@ var ( ErrorDesc(ErrGRPCFutureRev): ErrGRPCFutureRev, ErrorDesc(ErrGRPCNoSpace): ErrGRPCNoSpace, - ErrorDesc(ErrGRPCLeaseNotFound): ErrGRPCLeaseNotFound, - ErrorDesc(ErrGRPCLeaseExist): ErrGRPCLeaseExist, + ErrorDesc(ErrGRPCLeaseNotFound): ErrGRPCLeaseNotFound, + ErrorDesc(ErrGRPCLeaseExist): ErrGRPCLeaseExist, + ErrorDesc(ErrGRPCLeaseTTLTooLarge): ErrGRPCLeaseTTLTooLarge, ErrorDesc(ErrGRPCMemberExist): ErrGRPCMemberExist, ErrorDesc(ErrGRPCPeerURLExist): ErrGRPCPeerURLExist, @@ -131,8 +133,9 @@ var ( ErrFutureRev = Error(ErrGRPCFutureRev) ErrNoSpace = Error(ErrGRPCNoSpace) - ErrLeaseNotFound = Error(ErrGRPCLeaseNotFound) - ErrLeaseExist = Error(ErrGRPCLeaseExist) + ErrLeaseNotFound = Error(ErrGRPCLeaseNotFound) + ErrLeaseExist = Error(ErrGRPCLeaseExist) + ErrLeaseTTLTooLarge = Error(ErrGRPCLeaseTTLTooLarge) ErrMemberExist = Error(ErrGRPCMemberExist) ErrPeerURLExist = Error(ErrGRPCPeerURLExist) diff --git a/etcdserver/api/v3rpc/util.go b/etcdserver/api/v3rpc/util.go index 328135b1e67..799c1197d8d 100644 --- a/etcdserver/api/v3rpc/util.go +++ b/etcdserver/api/v3rpc/util.go @@ -52,8 +52,9 @@ var toGRPCErrorMap = map[error]error{ etcdserver.ErrKeyNotFound: rpctypes.ErrGRPCKeyNotFound, etcdserver.ErrCorrupt: rpctypes.ErrGRPCCorrupt, - lease.ErrLeaseNotFound: rpctypes.ErrGRPCLeaseNotFound, - lease.ErrLeaseExists: rpctypes.ErrGRPCLeaseExist, + lease.ErrLeaseNotFound: rpctypes.ErrGRPCLeaseNotFound, + lease.ErrLeaseExists: rpctypes.ErrGRPCLeaseExist, + lease.ErrLeaseTTLTooLarge: rpctypes.ErrGRPCLeaseTTLTooLarge, auth.ErrRootUserNotExist: rpctypes.ErrGRPCRootUserNotExist, auth.ErrRootRoleNotExist: rpctypes.ErrGRPCRootRoleNotExist, diff --git a/lease/lessor.go b/lease/lessor.go index 30dd2380693..31f645fa337 100644 --- a/lease/lessor.go +++ b/lease/lessor.go @@ -29,6 +29,9 @@ import ( // NoLease is a special LeaseID representing the absence of a lease. const NoLease = LeaseID(0) +// MaxLeaseTTL is the maximum lease TTL value +const MaxLeaseTTL = 9000000000 + var ( forever = time.Time{} @@ -37,9 +40,10 @@ var ( // maximum number of leases to revoke per second; configurable for tests leaseRevokeRate = 1000 - ErrNotPrimary = errors.New("not a primary lessor") - ErrLeaseNotFound = errors.New("lease not found") - ErrLeaseExists = errors.New("lease already exists") + ErrNotPrimary = errors.New("not a primary lessor") + ErrLeaseNotFound = errors.New("lease not found") + ErrLeaseExists = errors.New("lease already exists") + ErrLeaseTTLTooLarge = errors.New("too large lease TTL") ) // TxnDelete is a TxnWrite that only permits deletes. Defined here @@ -198,6 +202,10 @@ func (le *lessor) Grant(id LeaseID, ttl int64) (*Lease, error) { return nil, ErrLeaseNotFound } + if ttl > MaxLeaseTTL { + return nil, ErrLeaseTTLTooLarge + } + // TODO: when lessor is under high load, it should give out lease // with longer TTL to reduce renew load. l := &Lease{ diff --git a/lease/lessor_test.go b/lease/lessor_test.go index 3f6a5ce504b..3a39e846f72 100644 --- a/lease/lessor_test.go +++ b/lease/lessor_test.go @@ -451,6 +451,20 @@ func TestLessorExpireAndDemote(t *testing.T) { } } +func TestLessorMaxTTL(t *testing.T) { + dir, be := NewTestBackend(t) + defer os.RemoveAll(dir) + defer be.Close() + + le := newLessor(be, minLeaseTTL) + defer le.Stop() + + _, err := le.Grant(1, MaxLeaseTTL+1) + if err != ErrLeaseTTLTooLarge { + t.Fatalf("grant unexpectedly succeeded") + } +} + type fakeDeleter struct { deleted []string tx backend.BatchTx