[
https://issues.apache.org/jira/browse/ZOOKEEPER-4282?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17326440#comment-17326440
]
Damien Diederen commented on ZOOKEEPER-4282:
--------------------------------------------
Hi [~arshad.mohammad],
I was just looking into this.
Generally agree. Even when "hard" limits are set, the current quota
implementation
([ZOOKEEPER-3301|https://issues.apache.org/jira/browse/ZOOKEEPER-3301]) is
trivial to work around.
First, a few notes about your points, then a scary story below:
# Not necessarily against adding {{setQuota}} and friends, but wouldn't
creating all nodes in the {{/zookeeper/quota}} subtree with an ACL akin to
{{world:anyone:r}} (by default, value configurable) be technically sufficient,
and not require a change in protocol?
# ACL?
# I agree that the notion of an "application root node" seems to be quite
common in deployments, and that the native root should be protected in such
setups. Perhaps a simple configuration setting? Doing {{setAcl /
world:anyone:r}} as {{super}} is not that difficult, though---and the window is
probably negligible in practice;
# We currently have a tristate: {{enforceQuota:false}} means that quotas are
not being _processed_ at all; "soft" quotas cause overflows to be logged;
"hard" quotas cause requests to fail. (Not saying we need to preserve these
features; it was just to complete your description).
Now for the scary story:
The old quota implementation was supposed to be "advisory," but I looked a bit
deeper---and just noticed that besides its obvious limitations, the lack of
controls combined with the central location of the quota checks creates a
serious DoS vector!
(I was aware of a similar problem with
[ZOOKEEPER-451|https://issues.apache.org/jira/browse/ZOOKEEPER-451], but it
turns out that the issue is present in mainline 3.6 and earlier.)
On a "properly administered" ensemble, a {{super}} user sets up a "root node"
for user {{eve}}:
{noformat}
setAcl / world:anyone:r
create /eve
setAcl /eve sasl:eve:cdrwa
setquota /eve -B 32
{noformat}
Once logged in, {{eve}} can simply do:
{noformat}
set /zookeeper/quota/eve/zookeeper_limits boom
create /eve/was.here
{noformat}
which immediately causes the server to fail and exit with this nasty exception:
{noformat}
2021-04-21 12:20:25,861 [myid:] - ERROR
[SyncThread:0:ZooKeeperCriticalThread@49] - Severe unrecoverable error, from
thread : SyncThread:0
java.lang.IllegalArgumentException: invalid string yolo
at org.apache.zookeeper.StatsTrack.<init>(StatsTrack.java:50)
at
org.apache.zookeeper.server.DataTree.updateCountBytes(DataTree.java:409)
at org.apache.zookeeper.server.DataTree.createNode(DataTree.java:550)
{noformat}
Worse, the server won't restart before the corrupted data is excised from the
snapshot or transaction log.
This seems to be a minimal reproducer:
{noformat}
create /eve
create /zookeeper/quota/eve
create /zookeeper/quota/eve/zookeeper_stats boom
create /zookeeper/quota/eve/zookeeper_limits boom
create /eve/was.here
{noformat}
I would suggest opening another ticket, and creating PRs preventing the server
crash for 3.5 and 3.6. WDYT? Should I take care of it?
Best, -D
(Cc: [~eolivelli], [~maoling], [~hanm].)
> Redesign quota feature
> ----------------------
>
> Key: ZOOKEEPER-4282
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-4282
> Project: ZooKeeper
> Issue Type: New Feature
> Components: quota
> Reporter: Mohammad Arshad
> Assignee: Mohammad Arshad
> Priority: Major
> Fix For: 3.8.0
>
>
> *Quota Use Case:*
> Generally in a big data solution deployment multiple services (hdfs, yarn,
> hbase etc.) use single Zookeeper cluster. So it is very important to ensure
> fare usage by all services. Sometime services unintentionally, mainly because
> of faulty behavior, create many znodes and impact the overall reliability of
> the ZooKeeper service. To ensure the faire usage quota feature is required.
> But this is the only use case there are many other use cases for quota
> feature.
> *Current Problems:*
> # Currently, user can set quota by updating znode
> “/zookeeper/quota/nodepath”, or using setquota/delquota in CLI command.
> This makes the quota setting infective
> Currently any user can set/delete quota, which is not proper, it should be
> admin operation
> # User is allowed to modify zookeeper system paths like /zookeeper/quota.
> These are internal to zookeeper should not be allowed to modify.
> # Generally services create single top level znode in Zookeeper like /hbase
> and create all required znode under it.
> It is better if it is configurable who can create top level znodes to
> controll ZooKeeper usage.
> # After ZOOKEEPER-231, there two kinds quota enforcement limits 1. Hard limit
> 2. Soft limit.
> I think there should be only limit. When enforce quota is enabled that limits
> becomes the hard limit otherwise it is soft limit same as old feature, just
> logs warnings.
> *Proposed Solution*
> # Add setQuota and deleteQuota admin APIs. Add listQuota normal user API
> Modify quota cli commands to use these APIs instead of directory modifying
> ZooKeeper system path /zookeeper/quota/
> # Protect ZooKeeper system paths from outside modification. System should
> only be readable from outside
> # Expose configuration to set ACL for root system znode.
> After this, at the time of ZooKeeper service deployment administrator can
> create top level znode for a service and set quota. This way we can control
> overall ZooKeeper usage
> # Revert some of the changes in ZOOKEEPER-231 and move to single quota limit
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
