Details

    • Type: Umbrella Umbrella
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.94.1, 0.95.2
    • Fix Version/s: None
    • Component/s: security
    • Labels:
      None

      Description

      Umbrella issue for iteration on the initial AccessController drop.

        Issue Links

          Activity

          Hide
          Laxman added a comment -

          Please validate my understanding and correct me.

          1) Derived from HBASE-6061.
          Table Owner + Table CREATE = Table ADMIN

          2) Table ADMIN should be able to perform any operation on a table. (inline with GLOBAL ADMIN)
          Table ADMIN = CREATE + READ + WRITE + ADMIN PERMISSIONS

          ADMIN PERMISSIONS - Includes

          ADMIN should be able to perform operations like GET, SCAN, PUT and DELETE without explicit "READ" and "WRITE" permissions.

          3) Table WRITE permission is NOT an inclusion of Table READ.

          i.e., having WRITE permission alone indicates User is NOT authorized to do READ operations like GET/SCAN.

          Show
          Laxman added a comment - Please validate my understanding and correct me. 1) Derived from HBASE-6061 . Table Owner + Table CREATE = Table ADMIN 2) Table ADMIN should be able to perform any operation on a table. (inline with GLOBAL ADMIN) Table ADMIN = CREATE + READ + WRITE + ADMIN PERMISSIONS ADMIN PERMISSIONS - Includes ADMIN should be able to perform operations like GET, SCAN, PUT and DELETE without explicit "READ" and "WRITE" permissions. 3) Table WRITE permission is NOT an inclusion of Table READ. i.e., having WRITE permission alone indicates User is NOT authorized to do READ operations like GET/SCAN.
          Hide
          Matteo Bertozzi added a comment -

          1) no admin is different... able to do operation on the cluster (region move, unassign, ... and create/delete/modify all the tables)

          2) if you grant for 'A' you don't get RWC
          so admin are not able to read but are able to perform actions (create/delete/modify) on all tables

          3) if you grant 'W' you don't get 'R'

          The permission checks are done in this way:
          AccessController.permissionGranted()

          • Allow All to READ on .META. and ROOT
          • Allow Users with global ADMIN/CREATE to write on .META. (Add/Remove Table...)
          • Allow if user is Table Owner
          • Allow if user has Table Level rights
          • Allow if user has (Table) Family Level rights
          • Allow if user has (Table, Family) Qualifier Level rights
          Show
          Matteo Bertozzi added a comment - 1) no admin is different... able to do operation on the cluster (region move, unassign, ... and create/delete/modify all the tables) 2) if you grant for 'A' you don't get RWC so admin are not able to read but are able to perform actions (create/delete/modify) on all tables 3) if you grant 'W' you don't get 'R' The permission checks are done in this way: AccessController.permissionGranted() Allow All to READ on .META. and ROOT Allow Users with global ADMIN/CREATE to write on .META. (Add/Remove Table...) Allow if user is Table Owner Allow if user has Table Level rights Allow if user has (Table) Family Level rights Allow if user has (Table, Family) Qualifier Level rights
          Hide
          Andrew Purtell added a comment -

          Thanks Matteo, concur.

          IMO, it's preferable to conceptualize ADMIN permission as only an extra bit that allows you to interact with the Master on table management concerns.

          Show
          Andrew Purtell added a comment - Thanks Matteo, concur. IMO, it's preferable to conceptualize ADMIN permission as only an extra bit that allows you to interact with the Master on table management concerns.
          Hide
          Laxman added a comment -

          Thanks Matteo. I will try to come up with an ACL matrix with expected and actual permissions.
          Hope that helps in identifying all issues related to ACL.

          Show
          Laxman added a comment - Thanks Matteo. I will try to come up with an ACL matrix with expected and actual permissions. Hope that helps in identifying all issues related to ACL.
          Hide
          Matteo Bertozzi added a comment -
          • Admin
            • grant/revoke/get permission
            • create/delete/enable/disable all tables
            • move/assign/unassign/balance
            • shutdown/stop master
              @Laxman I already have this list... is not complete but can help you starting with your matrix
          • Create
            • create table
            • modify/delete/enable/disable table (if you’re table owner)
            • modify/delete/enable/disable table
            • add/modify/delete column
          • Write
            • put/delete/increment
          • Read
            • get/getClosestRow../exists/scan/
            • checkAndPut/checkAndDelete
          Show
          Matteo Bertozzi added a comment - Admin grant/revoke/get permission create/delete/enable/disable all tables move/assign/unassign/balance shutdown/stop master @Laxman I already have this list... is not complete but can help you starting with your matrix Create create table modify/delete/enable/disable table (if you’re table owner) modify/delete/enable/disable table add/modify/delete column Write put/delete/increment Read get/getClosestRow../exists/scan/ checkAndPut/checkAndDelete
          Hide
          Laxman added a comment -

          2) if you grant for 'A' you don't get RWC
          so admin are not able to read but are able to perform actions (create/delete/modify) on all tables

          IMO, it's preferable to conceptualize ADMIN permission as only an extra bit that allows you to interact with the Master on table management concerns.

          I agree. But I still need some details to prepare ACL matrix.

          There are two levels of 'A' (ADMIN) now.
          a) GLOBAL ADMIN (Super user - Configured using hbase.superuser)
          b) TABLE ADMIN (via grant)

          I'm not able to clearly differentiate between them.

          In the current implementation, GLOBAL ADMIN is able to read/write to any table. Which means, TABLE ADMIN will not be able to perform some operations on a table which GLOBAL ADMIN is able to do. Now we may need to discuss which part of these needs correction?

          IMO, GLOBAL ADMIN (for all tables) semantics should be inline with TABLE ADMIN (for one table).

          Show
          Laxman added a comment - 2) if you grant for 'A' you don't get RWC so admin are not able to read but are able to perform actions (create/delete/modify) on all tables IMO, it's preferable to conceptualize ADMIN permission as only an extra bit that allows you to interact with the Master on table management concerns. I agree. But I still need some details to prepare ACL matrix. There are two levels of 'A' (ADMIN) now. a) GLOBAL ADMIN (Super user - Configured using hbase.superuser) b) TABLE ADMIN (via grant) I'm not able to clearly differentiate between them. In the current implementation, GLOBAL ADMIN is able to read/write to any table. Which means, TABLE ADMIN will not be able to perform some operations on a table which GLOBAL ADMIN is able to do. Now we may need to discuss which part of these needs correction? IMO, GLOBAL ADMIN (for all tables) semantics should be inline with TABLE ADMIN (for one table).
          Hide
          Laxman added a comment -

          Read - checkAndPut/checkAndDelete

          @Matteo, I feel these two requires READ and WRITE together. No?

          Show
          Laxman added a comment - Read - checkAndPut/checkAndDelete @Matteo, I feel these two requires READ and WRITE together. No?
          Hide
          Matteo Bertozzi added a comment -

          @Laxman if you look at TableAuthManager there're two SkipList USER_CACHE and TABLE_USER_CACHE
          USER_CACHE contains the global rights, TABLE_USER_CACHE contains the table one.

          hbase.superuser is read at startup TableAuthManager.initGlobal() and fills the USER_CACHE with specified users with RWCA rights (so is not just ADMIN is RWCA)

          All CA operation are checked against global rights (USER_CACHE)
          so specifying CA in table rights has no effect.

          grant 'user', 'RWCA' -> fill USER_CACHE global rights
          grant 'user', 'RW', 'table' -> FILL TABLE_USER_CACHE

          this means if you're "global granted" (CREATE/ADMIN) you can add/modify/remove tables with the restriction that, if you've CREATE you can just modify/delete your own table while ADMIN you can modify/delete all the available tables.
          if you're table granted you can do just read/write depends on what is your permission, but you don't have the ability to create/modify/delete even if you've CA (CA are not checked against TABLE_USER_CACHE just USER_CACHE)

          for checkAndPut()/checkAndDelete() there's a jira open HBASE-6062 and yes it needs to check both Read and Write.

          Show
          Matteo Bertozzi added a comment - @Laxman if you look at TableAuthManager there're two SkipList USER_CACHE and TABLE_USER_CACHE USER_CACHE contains the global rights, TABLE_USER_CACHE contains the table one. hbase.superuser is read at startup TableAuthManager.initGlobal() and fills the USER_CACHE with specified users with RWCA rights (so is not just ADMIN is RWCA) All CA operation are checked against global rights (USER_CACHE) so specifying CA in table rights has no effect. grant 'user', 'RWCA' -> fill USER_CACHE global rights grant 'user', 'RW', 'table' -> FILL TABLE_USER_CACHE this means if you're "global granted" (CREATE/ADMIN) you can add/modify/remove tables with the restriction that, if you've CREATE you can just modify/delete your own table while ADMIN you can modify/delete all the available tables. if you're table granted you can do just read/write depends on what is your permission, but you don't have the ability to create/modify/delete even if you've CA (CA are not checked against TABLE_USER_CACHE just USER_CACHE) for checkAndPut()/checkAndDelete() there's a jira open HBASE-6062 and yes it needs to check both Read and Write.
          Hide
          Laxman added a comment -

          @Matteo, Thanks a lot for detailed explanation.
          With your above explanation, I understand that current design/implementation ignores TABLE CREATE/ADMIN permissions. But here, I'm trying to get more on "How it should be?" than "How it is currently?"

          Found related discussion (TABLE ADMIN vs GLOBAL ADMIN) @ HBASE-5372.
          I'm sorry if I'm asking basic questions.

          IMO, GLOBAL ADMIN (for all tables) semantics should be inline with TABLE ADMIN (for one table).

          Is this correct expectation or any different opinions?

          Show
          Laxman added a comment - @Matteo, Thanks a lot for detailed explanation. With your above explanation, I understand that current design/implementation ignores TABLE CREATE/ADMIN permissions. But here, I'm trying to get more on "How it should be?" than "How it is currently?" Found related discussion (TABLE ADMIN vs GLOBAL ADMIN) @ HBASE-5372 . I'm sorry if I'm asking basic questions. IMO, GLOBAL ADMIN (for all tables) semantics should be inline with TABLE ADMIN (for one table). Is this correct expectation or any different opinions?
          Hide
          Andrew Purtell added a comment -

          hbase.superuser is read at startup TableAuthManager.initGlobal() and fills the USER_CACHE with specified users with RWCA rights (so is not just ADMIN is RWCA)

          The "superuser" concept is a legacy from the initial implementation. Instead of having ADMIN rights mean something per user, there was/is an implicit grant of ADMIN rights to the superuser and that is it, for simplicity. So in our production the "hbase" user is used to manage the cluster by ops, and users have grants of only READ or WRITE as appropriate.

          GLOBAL ADMIN (for all tables) semantics should be inline with TABLE ADMIN (for one table).

          +1

          The "superuser" shortcut should be removed. Instead the AccessController could add a GLOBAL ADMIN grant on demand for the service principal of the master and regionservers when creating the ACL table. I don't think anyone is setting the "superuser" to anything other than the service principal.

          Also we could drop the "owner" concept (and table attribute) and instead have the AccessController add a TABLE ADMIN grant at table creation time, as discussed in HBASE-5372.

          Show
          Andrew Purtell added a comment - hbase.superuser is read at startup TableAuthManager.initGlobal() and fills the USER_CACHE with specified users with RWCA rights (so is not just ADMIN is RWCA) The "superuser" concept is a legacy from the initial implementation. Instead of having ADMIN rights mean something per user, there was/is an implicit grant of ADMIN rights to the superuser and that is it, for simplicity. So in our production the "hbase" user is used to manage the cluster by ops, and users have grants of only READ or WRITE as appropriate. GLOBAL ADMIN (for all tables) semantics should be inline with TABLE ADMIN (for one table). +1 The "superuser" shortcut should be removed. Instead the AccessController could add a GLOBAL ADMIN grant on demand for the service principal of the master and regionservers when creating the ACL table. I don't think anyone is setting the "superuser" to anything other than the service principal. Also we could drop the "owner" concept (and table attribute) and instead have the AccessController add a TABLE ADMIN grant at table creation time, as discussed in HBASE-5372 .
          Hide
          Andrew Purtell added a comment -

          checkAndPut/checkAndDelete

          These should need READ + WRITE.

          Show
          Andrew Purtell added a comment - checkAndPut/checkAndDelete These should need READ + WRITE.
          Hide
          Andrew Purtell added a comment - - edited

          And to clarify, the "superuser" can do anything, there is not only an implicit grant of ADMIN. Now that people are working on functioning global permissions it's not necessary to have this shortcut.

          Show
          Andrew Purtell added a comment - - edited And to clarify, the "superuser" can do anything, there is not only an implicit grant of ADMIN. Now that people are working on functioning global permissions it's not necessary to have this shortcut.
          Hide
          Enis Soztutar added a comment -

          The "superuser" shortcut should be removed.

          We need something like a superuser, so that if somehow there is a mixup of grants, we can fix it. But as Andrew suggest, just using the service principal should be good enough.

          Also we could drop the "owner" concept

          +1. It is better to manage all permissions from one place.

          Show
          Enis Soztutar added a comment - The "superuser" shortcut should be removed. We need something like a superuser, so that if somehow there is a mixup of grants, we can fix it. But as Andrew suggest, just using the service principal should be good enough. Also we could drop the "owner" concept +1. It is better to manage all permissions from one place.
          Hide
          Gary Helmling added a comment -

          The one possible downside to dropping the "owner" concept is that we lose any direct path to pushing down ownership to the table HFiles in HDFS.

          That said, I haven't seen any pressing need to push down ownership to HDFS, and the "owner" role was a bit of an odd exception in the permissions scheme, so I'm actually +1 on dropping it as well. Just wanted to raise this for completeness.

          Show
          Gary Helmling added a comment - The one possible downside to dropping the "owner" concept is that we lose any direct path to pushing down ownership to the table HFiles in HDFS. That said, I haven't seen any pressing need to push down ownership to HDFS, and the "owner" role was a bit of an odd exception in the permissions scheme, so I'm actually +1 on dropping it as well. Just wanted to raise this for completeness.
          Hide
          Laxman added a comment -

          Matteo:
          2) if you grant for 'A' you don't get RWC
          so admin are not able to read but are able to perform actions (create/delete/modify) on all tables

          Andrew:
          IMO, it's preferable to conceptualize ADMIN permission as only an extra bit that allows you to interact with the Master on table management concerns.

          @Matteo & Andy, thank you for your explanation.

          Yes, I couldn't agree to the point "ADMIN can't READ/WRITE".

          Say, GLOBAL/TABLE ADMIN should NOT be able to READ/WRITE from/to a table. But, they should be able to do ADMIN operations (including grant, revoke, etc.) on the table. Then, ADMIN can grant themselves any permission(CRW).

          It's an unwanted backdoor (may be a vulnerability). No?

          IMO, its no use in restricting ADMIN from READ/WRITE.

          Show
          Laxman added a comment - Matteo: 2) if you grant for 'A' you don't get RWC so admin are not able to read but are able to perform actions (create/delete/modify) on all tables Andrew: IMO, it's preferable to conceptualize ADMIN permission as only an extra bit that allows you to interact with the Master on table management concerns. @Matteo & Andy, thank you for your explanation. Yes, I couldn't agree to the point "ADMIN can't READ/WRITE". Say, GLOBAL/TABLE ADMIN should NOT be able to READ/WRITE from/to a table. But, they should be able to do ADMIN operations (including grant, revoke, etc.) on the table. Then, ADMIN can grant themselves any permission(CRW). It's an unwanted backdoor (may be a vulnerability). No? IMO, its no use in restricting ADMIN from READ/WRITE.
          Hide
          Andrew Purtell added a comment -

          IMO, its no use in restricting ADMIN from READ/WRITE.

          Though ADMIN can allow someone to grant themselves READ and/or WRITE permission, ADMIN should be a bit that allows DDL and table management ops, nothing to do with READ or WRITE permission. You raise a valid point Laxman but it's a documentation issue IMHO.

          Show
          Andrew Purtell added a comment - IMO, its no use in restricting ADMIN from READ/WRITE. Though ADMIN can allow someone to grant themselves READ and/or WRITE permission, ADMIN should be a bit that allows DDL and table management ops, nothing to do with READ or WRITE permission. You raise a valid point Laxman but it's a documentation issue IMHO.
          Hide
          Laxman added a comment -

          One observation:

          Revoke of Global permission is not taking effect without restart.
          Revoke is updating the acl table but it's not updating the USER_CACHE.

          Do we have any open issue for this or it needs a new jira?

          Show
          Laxman added a comment - One observation: Revoke of Global permission is not taking effect without restart. Revoke is updating the acl table but it's not updating the USER_CACHE. Do we have any open issue for this or it needs a new jira?
          Hide
          Andrew Purtell added a comment -

          Revoke of Global permission is not taking effect without restart.

          Better to make a new JIRA than reopen HBASE-5342 I'd say.

          Show
          Andrew Purtell added a comment - Revoke of Global permission is not taking effect without restart. Better to make a new JIRA than reopen HBASE-5342 I'd say.
          Hide
          Laxman added a comment -

          Thanks Andy. Added as a subtask HBASE-6157 to this parent jira. Hope that should be fine.

          Show
          Laxman added a comment - Thanks Andy. Added as a subtask HBASE-6157 to this parent jira. Hope that should be fine.
          Hide
          Laxman added a comment -

          Currently, concurrent access control operations (grant, revoke) may fail.
          Should we consider supporting concurrency for these operations in v2?

          IMO it's required if we wanted to claim HBase as secure.
          We don't have a use-case right now. It will be great if someone can provide, if any.

          Show
          Laxman added a comment - Currently, concurrent access control operations (grant, revoke) may fail. Should we consider supporting concurrency for these operations in v2? IMO it's required if we wanted to claim HBase as secure. We don't have a use-case right now. It will be great if someone can provide, if any.
          Hide
          Andrew Purtell added a comment -

          Currently, concurrent access control operations (grant, revoke) may fail. Should we consider supporting concurrency for these operations in v2?

          Maybe you can provide more context?

          Note that the permissions caches on each RS will synchronize with updates to the ACL table only after their ZK watches fire, which will be soon thereafter but not immediate. "Tightening" this means an ACL table read for every cluster operation, an instant hot spot, hence the design motivation for the caching. So for some short period after a grant, access may be denied; and, conversely, for some short period after a revoke, access may be allowed. This does not mean we cannot claim HBase as secure, because after ACLs are set up and the application goes production, it is enforced.

          Show
          Andrew Purtell added a comment - Currently, concurrent access control operations (grant, revoke) may fail. Should we consider supporting concurrency for these operations in v2? Maybe you can provide more context? Note that the permissions caches on each RS will synchronize with updates to the ACL table only after their ZK watches fire, which will be soon thereafter but not immediate. "Tightening" this means an ACL table read for every cluster operation, an instant hot spot, hence the design motivation for the caching. So for some short period after a grant, access may be denied; and, conversely, for some short period after a revoke, access may be allowed. This does not mean we cannot claim HBase as secure, because after ACLs are set up and the application goes production, it is enforced.
          Hide
          Laxman added a comment -

          So for some short period after a grant, access may be denied; and, conversely, for some short period after a revoke, access may be allowed.

          I agree with you on this Andy. But this is not my concern. I could have provide context.

          Maybe you can provide more context?

          In TableAuthManager, we may need to maintain thread-safe data structures.

          What I meant by concurrency was, when we execute access control operations(grant, revoke) and other user operation (scan, get, put) some calls may fail. Reasons for failure

          1) access control operations updates USER_CACHE & GROUP_CACHE
          2) user operation reads from USER_CACHE & GROUP_CACHE.

          And these data structures [ArrayListMultimap] are not thread-safe.

          http://google-collections.googlecode.com/svn/trunk/javadoc/com/google/common/collect/ArrayListMultimap.html

          Please correct me if I'm missing something here.

          Show
          Laxman added a comment - So for some short period after a grant, access may be denied; and, conversely, for some short period after a revoke, access may be allowed. I agree with you on this Andy. But this is not my concern. I could have provide context. Maybe you can provide more context? In TableAuthManager, we may need to maintain thread-safe data structures. What I meant by concurrency was, when we execute access control operations(grant, revoke) and other user operation (scan, get, put) some calls may fail. Reasons for failure 1) access control operations updates USER_CACHE & GROUP_CACHE 2) user operation reads from USER_CACHE & GROUP_CACHE. And these data structures [ArrayListMultimap] are not thread-safe. http://google-collections.googlecode.com/svn/trunk/javadoc/com/google/common/collect/ArrayListMultimap.html Please correct me if I'm missing something here.
          Hide
          Laxman added a comment -

          Attached the ACL matrix based on my understanding. Request for review and comments.

          Show
          Laxman added a comment - Attached the ACL matrix based on my understanding. Request for review and comments.
          Hide
          Andrew Purtell added a comment - - edited

          In TableAuthManager, we may need to maintain thread-safe data structures.

          So let's consider using a ReadWriteLock to protect for update. It is also possible to obtain a synchronized multimap with Multimaps#synchronizedListMultimap but by far the most common access is read access, and concurrent read access is fine.

          Edit: See HBASE-6186

          Show
          Andrew Purtell added a comment - - edited In TableAuthManager, we may need to maintain thread-safe data structures. So let's consider using a ReadWriteLock to protect for update. It is also possible to obtain a synchronized multimap with Multimaps#synchronizedListMultimap but by far the most common access is read access, and concurrent read access is fine. Edit: See HBASE-6186
          Hide
          Andrew Purtell added a comment -

          Attached the ACL matrix based on my understanding. Request for review and comments.

          Thanks for working this up Laxman.

          Elsewhere under this umbrella is discussion and some agreement on removing the concept of table owner. Instead during the process of creating a table the admin would grant as appropriate. What do you think?

          For splits, IMHO this does not need to change. Under what access control use case should we deny a region from splitting?

          For flushes, likewise, and this is an essential and time sensitive operation. Under what access control use case should we deny a region from flushing?

          For compaction, likewise.

          Everything else looks good to me. Perhaps others can comment?

          Show
          Andrew Purtell added a comment - Attached the ACL matrix based on my understanding. Request for review and comments. Thanks for working this up Laxman. Elsewhere under this umbrella is discussion and some agreement on removing the concept of table owner. Instead during the process of creating a table the admin would grant as appropriate. What do you think? For splits, IMHO this does not need to change. Under what access control use case should we deny a region from splitting? For flushes, likewise, and this is an essential and time sensitive operation. Under what access control use case should we deny a region from flushing? For compaction, likewise. Everything else looks good to me. Perhaps others can comment?
          Hide
          Laxman added a comment -

          Thanks for quick review and response Andy.

          +1 on removing the concept of table owner. Do we have any jira for this?

          split, flush, compact - these operations can be triggered internally or by user as well (via HBaseAdmin). Consider the following use-case.

          • User "A" owns table "T1"
          • User "B" owns table "T2"
          • Should we allow "B" to split/flush/compact "T2"?

          So, I feel these operations should be checked for "TABLE ADMIN/GLOBAL ADMIN". What you say?

          Show
          Laxman added a comment - Thanks for quick review and response Andy. +1 on removing the concept of table owner. Do we have any jira for this? split, flush, compact - these operations can be triggered internally or by user as well (via HBaseAdmin). Consider the following use-case. User "A" owns table "T1" User "B" owns table "T2" Should we allow "B" to split/flush/compact "T2"? So, I feel these operations should be checked for "TABLE ADMIN/GLOBAL ADMIN". What you say?
          Hide
          Andrew Purtell added a comment -

          I feel these operations should be checked for "TABLE ADMIN/GLOBAL ADMIN"

          As part of the processing of the administrative request, but therefore we need to hook it at the time of the RPC, not the split or compaction or flush itself. Pardon if we are talking about the same thing. Then I agree.

          Show
          Andrew Purtell added a comment - I feel these operations should be checked for "TABLE ADMIN/GLOBAL ADMIN" As part of the processing of the administrative request, but therefore we need to hook it at the time of the RPC, not the split or compaction or flush itself. Pardon if we are talking about the same thing. Then I agree.
          Hide
          Andrew Purtell added a comment -

          +1 on removing the concept of table owner. Do we have any jira for this?

          I'll open a subtask to be sure.

          Show
          Andrew Purtell added a comment - +1 on removing the concept of table owner. Do we have any jira for this? I'll open a subtask to be sure.
          Hide
          Matteo Bertozzi added a comment -

          ACL Matrix looks good to me, split & compact feels like just admin stuff.
          +1 on removing the table owner (setOwner not present in shell but just in HTableDescriptor)

          I don't understand why we need synchronize on _CACHE maps, the only one that update those maps is ZKPermissionWatcher... and the zookeeper events should be dispatched in a single thread. Am I missing something?

          Show
          Matteo Bertozzi added a comment - ACL Matrix looks good to me, split & compact feels like just admin stuff. +1 on removing the table owner (setOwner not present in shell but just in HTableDescriptor) I don't understand why we need synchronize on _CACHE maps, the only one that update those maps is ZKPermissionWatcher... and the zookeeper events should be dispatched in a single thread. Am I missing something?
          Hide
          Laxman added a comment -

          I don't understand why we need synchronize on _CACHE maps, the only one that update those maps is ZKPermissionWatcher... and the zookeeper events should be dispatched in a single thread. Am I missing something?

          @Matt, updates & reads from occurs in two multiple threads.

          1) access control operations(grant/revoke via ZKPermissionWatcher) updates USER_CACHE & GROUP_CACHE
          2) user operations(Any operation in matrix) reads from USER_CACHE & GROUP_CACHE.

          And these data structures [ArrayListMultimap] are not thread-safe.
          Hope this clarifies.

          Show
          Laxman added a comment - I don't understand why we need synchronize on _CACHE maps, the only one that update those maps is ZKPermissionWatcher... and the zookeeper events should be dispatched in a single thread. Am I missing something? @Matt, updates & reads from occurs in two multiple threads. 1) access control operations(grant/revoke via ZKPermissionWatcher) updates USER_CACHE & GROUP_CACHE 2) user operations(Any operation in matrix) reads from USER_CACHE & GROUP_CACHE. And these data structures [ArrayListMultimap] are not thread-safe. Hope this clarifies.
          Hide
          Matteo Bertozzi added a comment -

          And these data structures [ArrayListMultimap] are not thread-safe.

          Yep, looked at the code...

          So now we can have some problems on concurrent operations when you're granting user for a new table that is not in the map, and the map want to grow. (HBASE-6186)

          Show
          Matteo Bertozzi added a comment - And these data structures [ArrayListMultimap] are not thread-safe. Yep, looked at the code... So now we can have some problems on concurrent operations when you're granting user for a new table that is not in the map, and the map want to grow. ( HBASE-6186 )
          Hide
          Laxman added a comment -

          As part of the processing of the administrative request, but therefore we need to hook it at the time of the RPC, not the split or compaction or flush itself. Pardon if we are talking about the same thing. Then I agree.

          @Andy, do you mean we should not use available hooks [preSplit, preCompact, preFlush] and we should introduce new hooks.

          Can you please provide more details on this.

          I feel we should make use of the same coprocessor hooks.
          Security specific hooks in Coprocessor doesn't sounds correct to me.

          Please correct me if I misunderstood you.

          Show
          Laxman added a comment - As part of the processing of the administrative request, but therefore we need to hook it at the time of the RPC, not the split or compaction or flush itself. Pardon if we are talking about the same thing. Then I agree. @Andy, do you mean we should not use available hooks [preSplit, preCompact, preFlush] and we should introduce new hooks. Can you please provide more details on this. I feel we should make use of the same coprocessor hooks. Security specific hooks in Coprocessor doesn't sounds correct to me. Please correct me if I misunderstood you.
          Hide
          Andrew Purtell added a comment -

          Andy, do you mean we should not use available hooks [preSplit, preCompact, preFlush] and we should introduce new hooks.

          Lifecycle hooks (open, close, split, compact, flush) don't have the RPC context for determining "current user" so I don't see how we can differentiate between action initiated by the system or some user, or maybe you see some way to do that? Otherwise, we should insure the HBaseAdmin interface has full coverage and handle it there in the RPC context.

          Show
          Andrew Purtell added a comment - Andy, do you mean we should not use available hooks [preSplit, preCompact, preFlush] and we should introduce new hooks. Lifecycle hooks (open, close, split, compact, flush) don't have the RPC context for determining "current user" so I don't see how we can differentiate between action initiated by the system or some user, or maybe you see some way to do that? Otherwise, we should insure the HBaseAdmin interface has full coverage and handle it there in the RPC context.
          Hide
          Matteo Bertozzi added a comment -

          I don't see how we can differentiate between action initiated by the system or some user

          Note that you've the same problem when you're creating/deleting/modifying a table.
          You cannot differentiate between a user action and a system action... In this case create/delete/modify need to write on .META. so how can you limit user access on .META. keeping the ability to create/edit/delete a table?

          Show
          Matteo Bertozzi added a comment - I don't see how we can differentiate between action initiated by the system or some user Note that you've the same problem when you're creating/deleting/modifying a table. You cannot differentiate between a user action and a system action... In this case create/delete/modify need to write on .META. so how can you limit user access on .META. keeping the ability to create/edit/delete a table?
          Hide
          Andrew Purtell added a comment -

          @Matteo, I don't follow. Users must always be able to read from META and never be able to write.

          Show
          Andrew Purtell added a comment - @Matteo, I don't follow. Users must always be able to read from META and never be able to write.
          Hide
          Matteo Bertozzi added a comment -

          @Andrew when you a user create a table, a put is called on .META. to insert the table.
          Who is that user? He also need to be able to write on .META.

          Show
          Matteo Bertozzi added a comment - @Andrew when you a user create a table, a put is called on .META. to insert the table. Who is that user? He also need to be able to write on .META.
          Hide
          Andrew Purtell added a comment - - edited

          @Andrew when you a user create a table, a put is called on .META. to insert the table.

          Who is that user? He also need to be able to write on .META.

          The system principal / superuser. Like with system initiated compaction, split, region open, region close, etc.

          Show
          Andrew Purtell added a comment - - edited @Andrew when you a user create a table, a put is called on .META. to insert the table. Who is that user? He also need to be able to write on .META. The system principal / superuser. Like with system initiated compaction, split, region open, region close, etc.
          Hide
          Laxman added a comment -

          Thanks for your inputs Andy and Matt.
          I considered these points. please check the approach HBASE-6092.

          Show
          Laxman added a comment - Thanks for your inputs Andy and Matt. I considered these points. please check the approach HBASE-6092 .
          Hide
          Andrew Purtell added a comment -

          This umbrella has seen it's day. Resolving and spinning out still relevant subtasks to top level issues.

          Show
          Andrew Purtell added a comment - This umbrella has seen it's day. Resolving and spinning out still relevant subtasks to top level issues.

            People

            • Assignee:
              Unassigned
              Reporter:
              Andrew Purtell
            • Votes:
              1 Vote for this issue
              Watchers:
              15 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development