UserAdd and RoleAdd bypass --quota-backend-bytes

[CaojiamingAlan]

Bug report criteria

• This bug report is not security related, security issues should be disclosed privately via security@etcd.io.
• This is not a support request, support requests should be raised in the etcd discussion forums.
• You have read the etcd bug reporting guidelines.
• Existing open issues along with etcd frequently asked questions have been checked and this is not a duplicate.

What happened?

When the db size hit the limit of --quota-backend-bytes, users can still add users and roles, causing the db size continue to increase.

What did you expect to happen?

user add/role add should also return ErrNoSpace when hitting the limit

How can we reproduce it (as minimally and precisely as possible)?

1. Start etcd server with a low --quota-backend-bytes, for example 200kB: etcd --quota-backend-bytes=$((200*1024))

Check the db size with etcdctl --write-out=table endpoint status
The db's initial size is 98kB:

2. Use scripts to add 1000 users:

package main

import (
	"context"
	"fmt"
	clientv3 "go.etcd.io/etcd/client/v3"
	"google.golang.org/grpc"
	"math/rand"
	"time"
)

var letterRunes = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ")
func RandStringRunes(n int) string {
	b := make([]rune, n)
	for i := range b {
		b[i] = letterRunes[rand.Intn(len(letterRunes))]
	}
	return string(b)
}

func main() {
	fmt.Println("start")
	cli, err := clientv3.New(clientv3.Config{
		Endpoints:   []string{"localhost:2379"},
		DialTimeout: time.Second * 5,
		DialOptions: []grpc.DialOption{grpc.WithBlock()},
	})
	if err != nil {
		fmt.Println(err.Error())
		return
	}
	for i := 0; i < 1000; i++ {
		_, err := cli.UserAdd(context.Background(), RandStringRunes(100), "password")
		if err != nil {
			fmt.Println(err.Error())
			return
		}
	}
}

3. Check the db size, it is now 500kB, despite showing a NOSPACE alarm

Anything else we need to know?

No response

Etcd version (please run commands below)

I believe all versions are affected

Etcd configuration (command line flags or environment variables)

No response

Etcd debug information (please run commands below, feel free to obfuscate the IP address or FQDN in the output)

No response

Relevant log output

No response

[chaochn47]

Thanks @CaojiamingAlan for bringing it up!

Looks like it is missed not only in the quota applier but in grpc API layer.

[iuriatan]

It looks like a good first issue to me. If you think it's OK, I'd like to work on it and bring some update up until Wednesday, 5th.

[CaojiamingAlan]

@iuriatan Thank you for your enthusiasm. However, IMHO lets not rush to fix this.

In fact, the title of the issue may be a little bit misleading, since the set of auth related apis that need to be considered includes all the apis that write new records to the authstore(UserAdd/UserChangePassword/UserGrantRole/UserRevokeRole/RoleAdd/RoleGrantPermission/RoleRevokePermission etc.)

And for each api, it involves several parts of work:

1. Estimate the cost of each api as here
2. Implementing a new layer of grpc APIs as here
3. Override the apis for applierV3Capped and quotaApplierV3

All of the three parts do not have enough test coverage yet. Therefore, I don't think a PR that change all these manually would be accepted. We should improve the test coverage first and try to make changes after that. #16036 (comment)

I would also like to hear other's opinion about this.

[iuriatan]

@CaojiamingAlan Can you help defining some cases to be tested? I can work on extending appliers test coverage while the community discuss strategies.

[CaojiamingAlan]

The tests I've written so far covers the correctness of permission checks and alarm handling, but not yet throughly test the correctness of the result of applying. For example, first PUT and then RANGE to see whether the result matches. There are still some apis are not covered at all, like ClusterVersionSet.
You can run
go test -coverprofile coverage.out && go tool cover -html=coverage.out
under the server/etcdserver/apply folder to generate a report showing the test coverage.