[jira] [Commented] (ZOOKEEPER-2591) The deletion of Container znode doesn't check ACL delete permission
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16079263#comment-16079263 ] Jordan Zimmerman commented on ZOOKEEPER-2591: - I think preventing deleteContainer from clients is the best bet. We could even have a class of opcodes that are marked "internal only". > The deletion of Container znode doesn't check ACL delete permission > --- > > Key: ZOOKEEPER-2591 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2591 > Project: ZooKeeper > Issue Type: Bug > Components: security, server >Reporter: Edward Ribeiro >Assignee: Edward Ribeiro > > Container nodes check the ACL before creation, but the deletion doesn't check > the ACL rights. The code below succeeds even tough we removed ACL access > permissions for "/a". > {code} > zk.create("/a", null, Ids.OPEN_ACL_UNSAFE, CreateMode.CONTAINER); > ArrayList list = new ArrayList<>(); > list.add(new ACL(0, Ids.ANYONE_ID_UNSAFE)); > zk.setACL("/", list, -1); > zk.delete("/a", -1); > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (ZOOKEEPER-2591) The deletion of Container znode doesn't check ACL delete permission
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16079260#comment-16079260 ] Bhupendra Kumar Jain commented on ZOOKEEPER-2591: - As I understand, Request object (org.apache.zookeeper.server.Request) is created in server side only. Idea was to have boolean to indicate the type of request like system internal request or client request. Since this boolean will be set only by server so client can not control this. We can also do this by some other way like extend Request to create DeleteContainerRequest and check the request oject instance type in prepRequestProcessor. {quote} Another possibility is to somehow disallow OpCode.deleteContainer coming from a connected client. {quote} I agree your idea to disallow deleteContainer request from client completely. That way there is no need to add ACL check . I think we can check this in processPacket() method before submitting the request to request Processor. > The deletion of Container znode doesn't check ACL delete permission > --- > > Key: ZOOKEEPER-2591 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2591 > Project: ZooKeeper > Issue Type: Bug > Components: security, server >Reporter: Edward Ribeiro >Assignee: Edward Ribeiro > > Container nodes check the ACL before creation, but the deletion doesn't check > the ACL rights. The code below succeeds even tough we removed ACL access > permissions for "/a". > {code} > zk.create("/a", null, Ids.OPEN_ACL_UNSAFE, CreateMode.CONTAINER); > ArrayList list = new ArrayList<>(); > list.add(new ACL(0, Ids.ANYONE_ID_UNSAFE)); > zk.setACL("/", list, -1); > zk.delete("/a", -1); > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (ZOOKEEPER-2591) The deletion of Container znode doesn't check ACL delete permission
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16078251#comment-16078251 ] Jordan Zimmerman commented on ZOOKEEPER-2591: - [~ Bhupendra] - I don't understand how that would work. Any field that ContainerManager adds to the Request object could also be added by a rogue client. Can you give an example of how this would work? Another possibility is to someone disallow OpCode.deleteContainer coming from a connected client. > The deletion of Container znode doesn't check ACL delete permission > --- > > Key: ZOOKEEPER-2591 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2591 > Project: ZooKeeper > Issue Type: Bug > Components: security, server >Reporter: Edward Ribeiro >Assignee: Edward Ribeiro > > Container nodes check the ACL before creation, but the deletion doesn't check > the ACL rights. The code below succeeds even tough we removed ACL access > permissions for "/a". > {code} > zk.create("/a", null, Ids.OPEN_ACL_UNSAFE, CreateMode.CONTAINER); > ArrayList list = new ArrayList<>(); > list.add(new ACL(0, Ids.ANYONE_ID_UNSAFE)); > zk.setACL("/", list, -1); > zk.delete("/a", -1); > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (ZOOKEEPER-2591) The deletion of Container znode doesn't check ACL delete permission
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16077831#comment-16077831 ] Bhupendra Kumar Jain commented on ZOOKEEPER-2591: - Guys, Any thoughts on this ... > The deletion of Container znode doesn't check ACL delete permission > --- > > Key: ZOOKEEPER-2591 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2591 > Project: ZooKeeper > Issue Type: Bug > Components: security, server >Reporter: Edward Ribeiro >Assignee: Edward Ribeiro > > Container nodes check the ACL before creation, but the deletion doesn't check > the ACL rights. The code below succeeds even tough we removed ACL access > permissions for "/a". > {code} > zk.create("/a", null, Ids.OPEN_ACL_UNSAFE, CreateMode.CONTAINER); > ArrayList list = new ArrayList<>(); > list.add(new ACL(0, Ids.ANYONE_ID_UNSAFE)); > zk.setACL("/", list, -1); > zk.delete("/a", -1); > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (ZOOKEEPER-2591) The deletion of Container znode doesn't check ACL delete permission
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16073224#comment-16073224 ] Bhupendra Kumar Jain commented on ZOOKEEPER-2591: - About solution to have a check node.stat.getCversion() > 0 , I have one doubt . Lets consider a scenario as below 1. create container node 2. create child1 3. delete child1 cVersion = 1 , child count = 0 4. create child2 5. delete child2 6. cVersion = 2, child count = 0 7. Timer triggers and deletes container node What if malicious user deletes the container node after step 3. I agree that after step 3 even if system timer runs it will delete the container node too ... But then why allow malicious user to delete at all ... In my opinion we can have a internal Boolean in Request object which marked as true by ContainerManager and false for all other cases. We can skip the ACL check only if Boolean if true. Does it make sense ? > The deletion of Container znode doesn't check ACL delete permission > --- > > Key: ZOOKEEPER-2591 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2591 > Project: ZooKeeper > Issue Type: Bug > Components: security, server >Reporter: Edward Ribeiro >Assignee: Edward Ribeiro > > Container nodes check the ACL before creation, but the deletion doesn't check > the ACL rights. The code below succeeds even tough we removed ACL access > permissions for "/a". > {code} > zk.create("/a", null, Ids.OPEN_ACL_UNSAFE, CreateMode.CONTAINER); > ArrayList list = new ArrayList<>(); > list.add(new ACL(0, Ids.ANYONE_ID_UNSAFE)); > zk.setACL("/", list, -1); > zk.delete("/a", -1); > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (ZOOKEEPER-2591) The deletion of Container znode doesn't check ACL delete permission
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16073119#comment-16073119 ] Mohammad Arshad commented on ZOOKEEPER-2591: Adding "node.stat.getCversion() > 0" check makes sense to me. anybody submitting patch? I will review it. > The deletion of Container znode doesn't check ACL delete permission > --- > > Key: ZOOKEEPER-2591 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2591 > Project: ZooKeeper > Issue Type: Bug > Components: security, server >Reporter: Edward Ribeiro >Assignee: Edward Ribeiro > > Container nodes check the ACL before creation, but the deletion doesn't check > the ACL rights. The code below succeeds even tough we removed ACL access > permissions for "/a". > {code} > zk.create("/a", null, Ids.OPEN_ACL_UNSAFE, CreateMode.CONTAINER); > ArrayList list = new ArrayList<>(); > list.add(new ACL(0, Ids.ANYONE_ID_UNSAFE)); > zk.setACL("/", list, -1); > zk.delete("/a", -1); > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (ZOOKEEPER-2591) The deletion of Container znode doesn't check ACL delete permission
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16072626#comment-16072626 ] Edward Ribeiro commented on ZOOKEEPER-2591: --- Cool, got it. It is the only edge case previously described and your solution is nice. Excuse me for disturbing the talk with a spurious example. > The deletion of Container znode doesn't check ACL delete permission > --- > > Key: ZOOKEEPER-2591 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2591 > Project: ZooKeeper > Issue Type: Bug > Components: security, server >Reporter: Edward Ribeiro >Assignee: Edward Ribeiro > > Container nodes check the ACL before creation, but the deletion doesn't check > the ACL rights. The code below succeeds even tough we removed ACL access > permissions for "/a". > {code} > zk.create("/a", null, Ids.OPEN_ACL_UNSAFE, CreateMode.CONTAINER); > ArrayList list = new ArrayList<>(); > list.add(new ACL(0, Ids.ANYONE_ID_UNSAFE)); > zk.setACL("/", list, -1); > zk.delete("/a", -1); > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (ZOOKEEPER-2591) The deletion of Container znode doesn't check ACL delete permission
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16072606#comment-16072606 ] Jordan Zimmerman commented on ZOOKEEPER-2591: - container deletion, itself, is different yet. But, my point is that ZooKeeper clients expect containers to disappear so there's no real security risk. The only edge case I can see is a rogue client quickly deleting a container. We can fix that edge case by applying the logic as I describe above. > The deletion of Container znode doesn't check ACL delete permission > --- > > Key: ZOOKEEPER-2591 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2591 > Project: ZooKeeper > Issue Type: Bug > Components: security, server >Reporter: Edward Ribeiro >Assignee: Edward Ribeiro > > Container nodes check the ACL before creation, but the deletion doesn't check > the ACL rights. The code below succeeds even tough we removed ACL access > permissions for "/a". > {code} > zk.create("/a", null, Ids.OPEN_ACL_UNSAFE, CreateMode.CONTAINER); > ArrayList list = new ArrayList<>(); > list.add(new ACL(0, Ids.ANYONE_ID_UNSAFE)); > zk.setACL("/", list, -1); > zk.delete("/a", -1); > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (ZOOKEEPER-2591) The deletion of Container znode doesn't check ACL delete permission
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16072579#comment-16072579 ] Edward Ribeiro commented on ZOOKEEPER-2591: --- [~randgalt], oops, excuse me! I didn't look at this issue nor this part of the code since the issue was closed. > The deletion of Container znode doesn't check ACL delete permission > --- > > Key: ZOOKEEPER-2591 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2591 > Project: ZooKeeper > Issue Type: Bug > Components: security, server >Reporter: Edward Ribeiro >Assignee: Edward Ribeiro > > Container nodes check the ACL before creation, but the deletion doesn't check > the ACL rights. The code below succeeds even tough we removed ACL access > permissions for "/a". > {code} > zk.create("/a", null, Ids.OPEN_ACL_UNSAFE, CreateMode.CONTAINER); > ArrayList list = new ArrayList<>(); > list.add(new ACL(0, Ids.ANYONE_ID_UNSAFE)); > zk.setACL("/", list, -1); > zk.delete("/a", -1); > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (ZOOKEEPER-2591) The deletion of Container znode doesn't check ACL delete permission
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16072539#comment-16072539 ] Jordan Zimmerman commented on ZOOKEEPER-2591: - [~eribeiro] - I don't follow. The container node is created with an ACL. It uses the same create() method as normal node creation. A rogue client cannot delete child nodes without proper Auth. > The deletion of Container znode doesn't check ACL delete permission > --- > > Key: ZOOKEEPER-2591 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2591 > Project: ZooKeeper > Issue Type: Bug > Components: security, server >Reporter: Edward Ribeiro >Assignee: Edward Ribeiro > > Container nodes check the ACL before creation, but the deletion doesn't check > the ACL rights. The code below succeeds even tough we removed ACL access > permissions for "/a". > {code} > zk.create("/a", null, Ids.OPEN_ACL_UNSAFE, CreateMode.CONTAINER); > ArrayList list = new ArrayList<>(); > list.add(new ACL(0, Ids.ANYONE_ID_UNSAFE)); > zk.setACL("/", list, -1); > zk.delete("/a", -1); > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (ZOOKEEPER-2591) The deletion of Container znode doesn't check ACL delete permission
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16072530#comment-16072530 ] Edward Ribeiro commented on ZOOKEEPER-2591: --- [~randgalt], another edge case would be the following: a client create a container znode and populate it with children znodes. This client is the only in charge of deleting the children. Another clients should only be able to read the children znode contents, but a misbehaved client can delete children znodes in the container znode. With ACL properly set this other clients would not be able to delete the children of the master client. Does it make sense? > The deletion of Container znode doesn't check ACL delete permission > --- > > Key: ZOOKEEPER-2591 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2591 > Project: ZooKeeper > Issue Type: Bug > Components: security, server >Reporter: Edward Ribeiro >Assignee: Edward Ribeiro > > Container nodes check the ACL before creation, but the deletion doesn't check > the ACL rights. The code below succeeds even tough we removed ACL access > permissions for "/a". > {code} > zk.create("/a", null, Ids.OPEN_ACL_UNSAFE, CreateMode.CONTAINER); > ArrayList list = new ArrayList<>(); > list.add(new ACL(0, Ids.ANYONE_ID_UNSAFE)); > zk.setACL("/", list, -1); > zk.delete("/a", -1); > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (ZOOKEEPER-2591) The deletion of Container znode doesn't check ACL delete permission
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16071397#comment-16071397 ] Jordan Zimmerman commented on ZOOKEEPER-2591: - That's an extreme edge case but it is possible. We can prevent that by enforcing the container check of "node.stat.getCversion() > 0" - that would be a lot easier than adding an ACL check in PrepRequestProcessor's handling of OpCode.deleteContainer > The deletion of Container znode doesn't check ACL delete permission > --- > > Key: ZOOKEEPER-2591 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2591 > Project: ZooKeeper > Issue Type: Bug > Components: security, server >Reporter: Edward Ribeiro >Assignee: Edward Ribeiro > > Container nodes check the ACL before creation, but the deletion doesn't check > the ACL rights. The code below succeeds even tough we removed ACL access > permissions for "/a". > {code} > zk.create("/a", null, Ids.OPEN_ACL_UNSAFE, CreateMode.CONTAINER); > ArrayList list = new ArrayList<>(); > list.add(new ACL(0, Ids.ANYONE_ID_UNSAFE)); > zk.setACL("/", list, -1); > zk.delete("/a", -1); > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (ZOOKEEPER-2591) The deletion of Container znode doesn't check ACL delete permission
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16071390#comment-16071390 ] Bhupendra Kumar Jain commented on ZOOKEEPER-2591: - Right, But What if the malicious user deletes this node as soon as its get created. In that case the applications which tries to create children inside this parent node will fail. Its rare case but quite possible. > The deletion of Container znode doesn't check ACL delete permission > --- > > Key: ZOOKEEPER-2591 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2591 > Project: ZooKeeper > Issue Type: Bug > Components: security, server >Reporter: Edward Ribeiro >Assignee: Edward Ribeiro > > Container nodes check the ACL before creation, but the deletion doesn't check > the ACL rights. The code below succeeds even tough we removed ACL access > permissions for "/a". > {code} > zk.create("/a", null, Ids.OPEN_ACL_UNSAFE, CreateMode.CONTAINER); > ArrayList list = new ArrayList<>(); > list.add(new ACL(0, Ids.ANYONE_ID_UNSAFE)); > zk.setACL("/", list, -1); > zk.delete("/a", -1); > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (ZOOKEEPER-2591) The deletion of Container znode doesn't check ACL delete permission
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16070201#comment-16070201 ] Jordan Zimmerman commented on ZOOKEEPER-2591: - Yeah, I guess that could happen. IMO it isn't a big deal. ZooKeeper applications are expecting these nodes to disappear after a while. The server only deletes the node if it has no children. > The deletion of Container znode doesn't check ACL delete permission > --- > > Key: ZOOKEEPER-2591 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2591 > Project: ZooKeeper > Issue Type: Bug > Components: security, server >Reporter: Edward Ribeiro >Assignee: Edward Ribeiro > > Container nodes check the ACL before creation, but the deletion doesn't check > the ACL rights. The code below succeeds even tough we removed ACL access > permissions for "/a". > {code} > zk.create("/a", null, Ids.OPEN_ACL_UNSAFE, CreateMode.CONTAINER); > ArrayList list = new ArrayList<>(); > list.add(new ACL(0, Ids.ANYONE_ID_UNSAFE)); > zk.setACL("/", list, -1); > zk.delete("/a", -1); > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (ZOOKEEPER-2591) The deletion of Container znode doesn't check ACL delete permission
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16069679#comment-16069679 ] Bhupendra Kumar Jain commented on ZOOKEEPER-2591: - I agree that DeleteContainer client API is not provided but what If a malicious user creates the DeleteContainer request by his own and sends to server ? do server have any check to safeguard against this ? > The deletion of Container znode doesn't check ACL delete permission > --- > > Key: ZOOKEEPER-2591 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2591 > Project: ZooKeeper > Issue Type: Bug > Components: security, server >Reporter: Edward Ribeiro >Assignee: Edward Ribeiro > > Container nodes check the ACL before creation, but the deletion doesn't check > the ACL rights. The code below succeeds even tough we removed ACL access > permissions for "/a". > {code} > zk.create("/a", null, Ids.OPEN_ACL_UNSAFE, CreateMode.CONTAINER); > ArrayList list = new ArrayList<>(); > list.add(new ACL(0, Ids.ANYONE_ID_UNSAFE)); > zk.setACL("/", list, -1); > zk.delete("/a", -1); > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (ZOOKEEPER-2591) The deletion of Container znode doesn't check ACL delete permission
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16068397#comment-16068397 ] Jordan Zimmerman commented on ZOOKEEPER-2591: - If DeleteContainer had a client [~Bhupendra] API then ACL would make sense. But, the automatic version has no client associated with the operation and therefore there is no ACL/Auth to apply. > The deletion of Container znode doesn't check ACL delete permission > --- > > Key: ZOOKEEPER-2591 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2591 > Project: ZooKeeper > Issue Type: Bug > Components: security, server >Reporter: Edward Ribeiro >Assignee: Edward Ribeiro > > Container nodes check the ACL before creation, but the deletion doesn't check > the ACL rights. The code below succeeds even tough we removed ACL access > permissions for "/a". > {code} > zk.create("/a", null, Ids.OPEN_ACL_UNSAFE, CreateMode.CONTAINER); > ArrayList list = new ArrayList<>(); > list.add(new ACL(0, Ids.ANYONE_ID_UNSAFE)); > zk.setACL("/", list, -1); > zk.delete("/a", -1); > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (ZOOKEEPER-2591) The deletion of Container znode doesn't check ACL delete permission
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16068154#comment-16068154 ] Bhupendra Kumar Jain commented on ZOOKEEPER-2591: - IMO, OpCode.deleteContainer request can be initiated from client even though there is no explicit API in Zookeeper.java. In that case if ACL check is bypassed, node can be deleted by any user. So ACL check must be present if request is originated from client and ACL check can be skipped if request is system internal. > The deletion of Container znode doesn't check ACL delete permission > --- > > Key: ZOOKEEPER-2591 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2591 > Project: ZooKeeper > Issue Type: Bug > Components: security, server >Reporter: Edward Ribeiro >Assignee: Edward Ribeiro > > Container nodes check the ACL before creation, but the deletion doesn't check > the ACL rights. The code below succeeds even tough we removed ACL access > permissions for "/a". > {code} > zk.create("/a", null, Ids.OPEN_ACL_UNSAFE, CreateMode.CONTAINER); > ArrayList list = new ArrayList<>(); > list.add(new ACL(0, Ids.ANYONE_ID_UNSAFE)); > zk.setACL("/", list, -1); > zk.delete("/a", -1); > {code} -- This message was sent by Atlassian JIRA (v6.4.14#64029)