anmolnar commented on PR #1298: URL: https://github.com/apache/zookeeper/pull/1298#issuecomment-1812613937
@maoling @eolivelli Would you mind resurrecting this patch? Patch looks good to me, but it's not enough alone to fix the original issue. Currently `exists()` doesn't check ACL, so nodes can be easily 'scanned' for existing even if the user doesn't have permission to the znode or the parent znode. For instance: ``` /a - protected /a/b - protected /a/c - not protected ``` In this case an unauthorized user should not be able to see anything under `/a`, because we check ACL in `getChildren()`. With the current impl of `exists()`, he can scan through nodes `/a/a`, `/a/b`, `/a/c`, etc. to see which one exists. This still can be done with this patch, because he will get: ``` stat /a - Insuff perm stat /a/b - Insuff perm stat /a/c - Node does not exist ``` I suggest the following: - Check nearest parent znode ACL in exists(), which is `/a` in my example and it's also `/a` for `stat /a/b/d/e/f/g/h`. In this case he will get Insuff perm for all the above requests. - Put this behind a feature flag and let the default behaviour of `exists()` unprotected (current situation). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@zookeeper.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
