You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Aleksey Yeschenko (JIRA)" <ji...@apache.org> on 2012/10/30 05:06:12 UTC

[jira] [Created] (CASSANDRA-4875) Possible improvements to IAuthority[2] interface

Aleksey Yeschenko created CASSANDRA-4875:
--------------------------------------------

             Summary: Possible improvements to IAuthority[2] interface
                 Key: CASSANDRA-4875
                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4875
             Project: Cassandra
          Issue Type: Improvement
    Affects Versions: 1.2.0 beta 1, 1.1.6
            Reporter: Aleksey Yeschenko
            Assignee: Aleksey Yeschenko


CASSANDRA-4874 is about general improvements to authorization handling, this one is about IAuthority[2] in particular.

- 'LIST GRANTS OF user should' become 'LIST PERMISSIONS [on resource] [of user]'.
Currently there is no way to see all the permissions on the resource, only all the permissions of a particular user.
- IAuthority2.listPermissions() should return a generic collection of ResoucePermission or something, not CQLResult or ResultMessage.
That's a wrong level of abstraction. I know this issue has been raised here - https://issues.apache.org/jira/browse/CASSANDRA-4490?focusedCommentId=13449732&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732, but I think it's possible to change this. Returning a list of {resource, user, permission, grant_option} tuples should be possible.
- We should get rid of Permission.NO_ACCESS. An empty list of permissions should mean absence of any permission, not some magical Permission.NO_ACCESS value.
It's insecure and error-prone and also ambiguous (what if a user has both FULL_ACCESS and NO_ACCESS permissions? If it's meant to be a way to strip a user
of all permissions on the resource, then it should be replaced with some form of REVOKE statement. Something like 'REVOKE ALL PERMISSIONS' sounds more logical than GRANT NO_ACCESS to me.
- Previous point will probably require adding revokeAllPermissions() method to make it explicit, special-casing IAuthority2.revoke() won't do
- IAuthorize2.grant() and IAuthorize2.revoke() accept CFName instance for a resource, which has its ks and cf fields swapped if cf is omitted. This may cause a real security issue if IAuthorize2 implementer doesn't know about the issue. We must pass the resouce as a collection of strings ([cassandra, keyspaces[, ks_name][, cf_name]]) instead, the way we pass it to IAuthorize.authorize().
- We should probably get rid of FULL_ACCESS as well, at least as a valid permission value (but maybe allow it in the CQL statement) and add an equivalent IAuthority2.grantAllPermissions(), separately. Why? Imagine the following sequence: GRANT FULL_ACCESS ON resource FOR user; REVOKE SELECT ON resource FROM user; should the user be allowed to SELECT anymore?
I say no, he shouldn't. Full access should be represented by a list of all permissions, not by a magical special value.
- P.DELETE probably should go in favour of P.UPDATE even for TRUNCATE. Presence of P.DELETE will definitely confuse users, who might think that it is somehow required to delete data, when it isn't. You can overwrite every value if you have P.UPDATE with TTL=1 and get the same result. We should also drop P.INSERT. Leave P.UPDATE (or rename it to P.MODIFY). P.MODIFY_DATA + P.READ_DATA should replace P.UPDATE, P.SELECT and P.DELETE.
- I suggest new syntax to allow setting permissions on cassandra/keyspaces resource: GRANT <permission> ON * FOR <user>.

The interface has to change because of the CFName argument to grant() and revoke(), and since it's going to be broken anyway (and has been introduced recently), I think we are in a position to make some other improvements while at it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4875) Possible improvements to IAuthority[2] interface

Posted by "Aleksey Yeschenko (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13488992#comment-13488992 ] 

Aleksey Yeschenko commented on CASSANDRA-4875:
----------------------------------------------

What about requiring the full name of the resource, without the option to omit parts of the hierarchy? What about the syntax itself ('fully/qualified/name') ?
Do you agree with GRANT ALL ON mapping to "for permission in listPermissions(): grant(permission)" and getting rid of P.FULL_ACCESS? And I don't understand whether "'LIST PERMISSIONS [of user] [on resource]' is slightly more grammatical" is a yes or a no.
Otherwise yes, we covered everything.
                
> Possible improvements to IAuthority[2] interface
> ------------------------------------------------
>
>                 Key: CASSANDRA-4875
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4875
>             Project: Cassandra
>          Issue Type: Improvement
>    Affects Versions: 1.1.6, 1.2.0 beta 1
>            Reporter: Aleksey Yeschenko
>            Assignee: Aleksey Yeschenko
>              Labels: security
>
> CASSANDRA-4874 is about general improvements to authorization handling, this one is about IAuthority[2] in particular.
> - 'LIST GRANTS OF user should' become 'LIST PERMISSIONS [on resource] [of user]'.
> Currently there is no way to see all the permissions on the resource, only all the permissions of a particular user.
> - IAuthority2.listPermissions() should return a generic collection of ResoucePermission or something, not CQLResult or ResultMessage.
> That's a wrong level of abstraction. I know this issue has been raised here - https://issues.apache.org/jira/browse/CASSANDRA-4490?focusedCommentId=13449732&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732, but I think it's possible to change this. Returning a list of {resource, user, permission, grant_option} tuples should be possible.
> - We should get rid of Permission.NO_ACCESS. An empty list of permissions should mean absence of any permission, not some magical Permission.NO_ACCESS value.
> It's insecure and error-prone and also ambiguous (what if a user has both FULL_ACCESS and NO_ACCESS permissions? If it's meant to be a way to strip a user
> of all permissions on the resource, then it should be replaced with some form of REVOKE statement. Something like 'REVOKE ALL PERMISSIONS' sounds more logical than GRANT NO_ACCESS to me.
> - Previous point will probably require adding revokeAllPermissions() method to make it explicit, special-casing IAuthority2.revoke() won't do
> - IAuthorize2.grant() and IAuthorize2.revoke() accept CFName instance for a resource, which has its ks and cf fields swapped if cf is omitted. This may cause a real security issue if IAuthorize2 implementer doesn't know about the issue. We must pass the resouce as a collection of strings ([cassandra, keyspaces[, ks_name][, cf_name]]) instead, the way we pass it to IAuthorize.authorize().
> - We should probably get rid of FULL_ACCESS as well, at least as a valid permission value (but maybe allow it in the CQL statement) and add an equivalent IAuthority2.grantAllPermissions(), separately. Why? Imagine the following sequence: GRANT FULL_ACCESS ON resource FOR user; REVOKE SELECT ON resource FROM user; should the user be allowed to SELECT anymore?
> I say no, he shouldn't. Full access should be represented by a list of all permissions, not by a magical special value.
> - P.DELETE probably should go in favour of P.UPDATE even for TRUNCATE. Presence of P.DELETE will definitely confuse users, who might think that it is somehow required to delete data, when it isn't. You can overwrite every value if you have P.UPDATE with TTL=1 and get the same result. We should also drop P.INSERT. Leave P.UPDATE (or rename it to P.MODIFY). P.MODIFY_DATA + P.READ_DATA should replace P.UPDATE, P.SELECT and P.DELETE.
> - I suggest new syntax to allow setting permissions on cassandra/keyspaces resource: GRANT <permission> ON * FOR <user>.
> The interface has to change because of the CFName argument to grant() and revoke(), and since it's going to be broken anyway (and has been introduced recently), I think we are in a position to make some other improvements while at it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (CASSANDRA-4875) Possible improvements to IAuthority[2] interface

Posted by "Aleksey Yeschenko (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CASSANDRA-4875?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Aleksey Yeschenko updated CASSANDRA-4875:
-----------------------------------------

    Reviewer: jbellis
    
> Possible improvements to IAuthority[2] interface
> ------------------------------------------------
>
>                 Key: CASSANDRA-4875
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4875
>             Project: Cassandra
>          Issue Type: Improvement
>    Affects Versions: 1.1.6, 1.2.0 beta 1
>            Reporter: Aleksey Yeschenko
>            Assignee: Aleksey Yeschenko
>              Labels: security
>             Fix For: 1.2.0 rc1
>
>         Attachments: 0001-Rip-out-IAuthority2-from-1.1.txt
>
>
> CASSANDRA-4874 is about general improvements to authorization handling, this one is about IAuthority[2] in particular.
> - 'LIST GRANTS OF user should' become 'LIST PERMISSIONS [on resource] [of user]'.
> Currently there is no way to see all the permissions on the resource, only all the permissions of a particular user.
> - IAuthority2.listPermissions() should return a generic collection of ResoucePermission or something, not CQLResult or ResultMessage.
> That's a wrong level of abstraction. I know this issue has been raised here - https://issues.apache.org/jira/browse/CASSANDRA-4490?focusedCommentId=13449732&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732, but I think it's possible to change this. Returning a list of {resource, user, permission, grant_option} tuples should be possible.
> - We should get rid of Permission.NO_ACCESS. An empty list of permissions should mean absence of any permission, not some magical Permission.NO_ACCESS value.
> It's insecure and error-prone and also ambiguous (what if a user has both FULL_ACCESS and NO_ACCESS permissions? If it's meant to be a way to strip a user
> of all permissions on the resource, then it should be replaced with some form of REVOKE statement. Something like 'REVOKE ALL PERMISSIONS' sounds more logical than GRANT NO_ACCESS to me.
> - Previous point will probably require adding revokeAllPermissions() method to make it explicit, special-casing IAuthority2.revoke() won't do
> - IAuthorize2.grant() and IAuthorize2.revoke() accept CFName instance for a resource, which has its ks and cf fields swapped if cf is omitted. This may cause a real security issue if IAuthorize2 implementer doesn't know about the issue. We must pass the resouce as a collection of strings ([cassandra, keyspaces[, ks_name][, cf_name]]) instead, the way we pass it to IAuthorize.authorize().
> - We should probably get rid of FULL_ACCESS as well, at least as a valid permission value (but maybe allow it in the CQL statement) and add an equivalent IAuthority2.grantAllPermissions(), separately. Why? Imagine the following sequence: GRANT FULL_ACCESS ON resource FOR user; REVOKE SELECT ON resource FROM user; should the user be allowed to SELECT anymore?
> I say no, he shouldn't. Full access should be represented by a list of all permissions, not by a magical special value.
> - P.DELETE probably should go in favour of P.UPDATE even for TRUNCATE. Presence of P.DELETE will definitely confuse users, who might think that it is somehow required to delete data, when it isn't. You can overwrite every value if you have P.UPDATE with TTL=1 and get the same result. We should also drop P.INSERT. Leave P.UPDATE (or rename it to P.MODIFY). P.MODIFY_DATA + P.READ_DATA should replace P.UPDATE, P.SELECT and P.DELETE.
> - I suggest new syntax to allow setting permissions on cassandra/keyspaces resource: GRANT <permission> ON * FOR <user>.
> The interface has to change because of the CFName argument to grant() and revoke(), and since it's going to be broken anyway (and has been introduced recently), I think we are in a position to make some other improvements while at it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (CASSANDRA-4875) Possible improvements to IAuthority[2] interface

Posted by "Aleksey Yeschenko (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CASSANDRA-4875?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Aleksey Yeschenko updated CASSANDRA-4875:
-----------------------------------------

    Attachment: 0001-Rip-out-IAuthority2-from-1.1.txt
    
> Possible improvements to IAuthority[2] interface
> ------------------------------------------------
>
>                 Key: CASSANDRA-4875
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4875
>             Project: Cassandra
>          Issue Type: Improvement
>    Affects Versions: 1.1.6, 1.2.0 beta 1
>            Reporter: Aleksey Yeschenko
>            Assignee: Aleksey Yeschenko
>              Labels: security
>             Fix For: 1.2.0 rc1
>
>         Attachments: 0001-Rip-out-IAuthority2-from-1.1.txt
>
>
> CASSANDRA-4874 is about general improvements to authorization handling, this one is about IAuthority[2] in particular.
> - 'LIST GRANTS OF user should' become 'LIST PERMISSIONS [on resource] [of user]'.
> Currently there is no way to see all the permissions on the resource, only all the permissions of a particular user.
> - IAuthority2.listPermissions() should return a generic collection of ResoucePermission or something, not CQLResult or ResultMessage.
> That's a wrong level of abstraction. I know this issue has been raised here - https://issues.apache.org/jira/browse/CASSANDRA-4490?focusedCommentId=13449732&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732, but I think it's possible to change this. Returning a list of {resource, user, permission, grant_option} tuples should be possible.
> - We should get rid of Permission.NO_ACCESS. An empty list of permissions should mean absence of any permission, not some magical Permission.NO_ACCESS value.
> It's insecure and error-prone and also ambiguous (what if a user has both FULL_ACCESS and NO_ACCESS permissions? If it's meant to be a way to strip a user
> of all permissions on the resource, then it should be replaced with some form of REVOKE statement. Something like 'REVOKE ALL PERMISSIONS' sounds more logical than GRANT NO_ACCESS to me.
> - Previous point will probably require adding revokeAllPermissions() method to make it explicit, special-casing IAuthority2.revoke() won't do
> - IAuthorize2.grant() and IAuthorize2.revoke() accept CFName instance for a resource, which has its ks and cf fields swapped if cf is omitted. This may cause a real security issue if IAuthorize2 implementer doesn't know about the issue. We must pass the resouce as a collection of strings ([cassandra, keyspaces[, ks_name][, cf_name]]) instead, the way we pass it to IAuthorize.authorize().
> - We should probably get rid of FULL_ACCESS as well, at least as a valid permission value (but maybe allow it in the CQL statement) and add an equivalent IAuthority2.grantAllPermissions(), separately. Why? Imagine the following sequence: GRANT FULL_ACCESS ON resource FOR user; REVOKE SELECT ON resource FROM user; should the user be allowed to SELECT anymore?
> I say no, he shouldn't. Full access should be represented by a list of all permissions, not by a magical special value.
> - P.DELETE probably should go in favour of P.UPDATE even for TRUNCATE. Presence of P.DELETE will definitely confuse users, who might think that it is somehow required to delete data, when it isn't. You can overwrite every value if you have P.UPDATE with TTL=1 and get the same result. We should also drop P.INSERT. Leave P.UPDATE (or rename it to P.MODIFY). P.MODIFY_DATA + P.READ_DATA should replace P.UPDATE, P.SELECT and P.DELETE.
> - I suggest new syntax to allow setting permissions on cassandra/keyspaces resource: GRANT <permission> ON * FOR <user>.
> The interface has to change because of the CFName argument to grant() and revoke(), and since it's going to be broken anyway (and has been introduced recently), I think we are in a position to make some other improvements while at it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4875) Possible improvements to IAuthority[2] interface

Posted by "Aleksey Yeschenko (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13488860#comment-13488860 ] 

Aleksey Yeschenko commented on CASSANDRA-4875:
----------------------------------------------

bq.  The problem with requiring permissions to be enumerated is, you add a new CF and now none of your GRANT ALL users have access to it unless you spell it out for each. Painful. (And not the way other databases work.)
I don't see how this is true. All I'm proposing is to make FULL_ACCESS the opposite of NO_ACCESS.
REVOKE ALL ON ..: "for permission in listPermissions(): revoke(permission)"
GRANT ALL ON ..: "for permission in listPermissions(): grant(permission)"
If you create a new CF then your (ks level) GRANT ALL users will have complete access to it. As long as we respect the hierarchy. It doesn't matter in this case whether the have a single FULL_ACCESS on the KS or ALTER, SELECT, MODIFY.. etc. on the KS - the result is the same.
                
> Possible improvements to IAuthority[2] interface
> ------------------------------------------------
>
>                 Key: CASSANDRA-4875
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4875
>             Project: Cassandra
>          Issue Type: Improvement
>    Affects Versions: 1.1.6, 1.2.0 beta 1
>            Reporter: Aleksey Yeschenko
>            Assignee: Aleksey Yeschenko
>              Labels: security
>
> CASSANDRA-4874 is about general improvements to authorization handling, this one is about IAuthority[2] in particular.
> - 'LIST GRANTS OF user should' become 'LIST PERMISSIONS [on resource] [of user]'.
> Currently there is no way to see all the permissions on the resource, only all the permissions of a particular user.
> - IAuthority2.listPermissions() should return a generic collection of ResoucePermission or something, not CQLResult or ResultMessage.
> That's a wrong level of abstraction. I know this issue has been raised here - https://issues.apache.org/jira/browse/CASSANDRA-4490?focusedCommentId=13449732&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732, but I think it's possible to change this. Returning a list of {resource, user, permission, grant_option} tuples should be possible.
> - We should get rid of Permission.NO_ACCESS. An empty list of permissions should mean absence of any permission, not some magical Permission.NO_ACCESS value.
> It's insecure and error-prone and also ambiguous (what if a user has both FULL_ACCESS and NO_ACCESS permissions? If it's meant to be a way to strip a user
> of all permissions on the resource, then it should be replaced with some form of REVOKE statement. Something like 'REVOKE ALL PERMISSIONS' sounds more logical than GRANT NO_ACCESS to me.
> - Previous point will probably require adding revokeAllPermissions() method to make it explicit, special-casing IAuthority2.revoke() won't do
> - IAuthorize2.grant() and IAuthorize2.revoke() accept CFName instance for a resource, which has its ks and cf fields swapped if cf is omitted. This may cause a real security issue if IAuthorize2 implementer doesn't know about the issue. We must pass the resouce as a collection of strings ([cassandra, keyspaces[, ks_name][, cf_name]]) instead, the way we pass it to IAuthorize.authorize().
> - We should probably get rid of FULL_ACCESS as well, at least as a valid permission value (but maybe allow it in the CQL statement) and add an equivalent IAuthority2.grantAllPermissions(), separately. Why? Imagine the following sequence: GRANT FULL_ACCESS ON resource FOR user; REVOKE SELECT ON resource FROM user; should the user be allowed to SELECT anymore?
> I say no, he shouldn't. Full access should be represented by a list of all permissions, not by a magical special value.
> - P.DELETE probably should go in favour of P.UPDATE even for TRUNCATE. Presence of P.DELETE will definitely confuse users, who might think that it is somehow required to delete data, when it isn't. You can overwrite every value if you have P.UPDATE with TTL=1 and get the same result. We should also drop P.INSERT. Leave P.UPDATE (or rename it to P.MODIFY). P.MODIFY_DATA + P.READ_DATA should replace P.UPDATE, P.SELECT and P.DELETE.
> - I suggest new syntax to allow setting permissions on cassandra/keyspaces resource: GRANT <permission> ON * FOR <user>.
> The interface has to change because of the CFName argument to grant() and revoke(), and since it's going to be broken anyway (and has been introduced recently), I think we are in a position to make some other improvements while at it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4875) Possible improvements to IAuthority[2] interface

Posted by "Aleksey Yeschenko (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13488868#comment-13488868 ] 

Aleksey Yeschenko commented on CASSANDRA-4875:
----------------------------------------------

This will also simplify permission checks - no need to check for both the requested permission AND FULL_ACCESS. If you need P.CREATE - just look for P.CREATE on every level of the hierarchy and that's all. Currently it's really complicated, since we also still have NO_ACCESS.
                
> Possible improvements to IAuthority[2] interface
> ------------------------------------------------
>
>                 Key: CASSANDRA-4875
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4875
>             Project: Cassandra
>          Issue Type: Improvement
>    Affects Versions: 1.1.6, 1.2.0 beta 1
>            Reporter: Aleksey Yeschenko
>            Assignee: Aleksey Yeschenko
>              Labels: security
>
> CASSANDRA-4874 is about general improvements to authorization handling, this one is about IAuthority[2] in particular.
> - 'LIST GRANTS OF user should' become 'LIST PERMISSIONS [on resource] [of user]'.
> Currently there is no way to see all the permissions on the resource, only all the permissions of a particular user.
> - IAuthority2.listPermissions() should return a generic collection of ResoucePermission or something, not CQLResult or ResultMessage.
> That's a wrong level of abstraction. I know this issue has been raised here - https://issues.apache.org/jira/browse/CASSANDRA-4490?focusedCommentId=13449732&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732, but I think it's possible to change this. Returning a list of {resource, user, permission, grant_option} tuples should be possible.
> - We should get rid of Permission.NO_ACCESS. An empty list of permissions should mean absence of any permission, not some magical Permission.NO_ACCESS value.
> It's insecure and error-prone and also ambiguous (what if a user has both FULL_ACCESS and NO_ACCESS permissions? If it's meant to be a way to strip a user
> of all permissions on the resource, then it should be replaced with some form of REVOKE statement. Something like 'REVOKE ALL PERMISSIONS' sounds more logical than GRANT NO_ACCESS to me.
> - Previous point will probably require adding revokeAllPermissions() method to make it explicit, special-casing IAuthority2.revoke() won't do
> - IAuthorize2.grant() and IAuthorize2.revoke() accept CFName instance for a resource, which has its ks and cf fields swapped if cf is omitted. This may cause a real security issue if IAuthorize2 implementer doesn't know about the issue. We must pass the resouce as a collection of strings ([cassandra, keyspaces[, ks_name][, cf_name]]) instead, the way we pass it to IAuthorize.authorize().
> - We should probably get rid of FULL_ACCESS as well, at least as a valid permission value (but maybe allow it in the CQL statement) and add an equivalent IAuthority2.grantAllPermissions(), separately. Why? Imagine the following sequence: GRANT FULL_ACCESS ON resource FOR user; REVOKE SELECT ON resource FROM user; should the user be allowed to SELECT anymore?
> I say no, he shouldn't. Full access should be represented by a list of all permissions, not by a magical special value.
> - P.DELETE probably should go in favour of P.UPDATE even for TRUNCATE. Presence of P.DELETE will definitely confuse users, who might think that it is somehow required to delete data, when it isn't. You can overwrite every value if you have P.UPDATE with TTL=1 and get the same result. We should also drop P.INSERT. Leave P.UPDATE (or rename it to P.MODIFY). P.MODIFY_DATA + P.READ_DATA should replace P.UPDATE, P.SELECT and P.DELETE.
> - I suggest new syntax to allow setting permissions on cassandra/keyspaces resource: GRANT <permission> ON * FOR <user>.
> The interface has to change because of the CFName argument to grant() and revoke(), and since it's going to be broken anyway (and has been introduced recently), I think we are in a position to make some other improvements while at it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (CASSANDRA-4875) Possible improvements to IAuthority[2] interface

Posted by "Aleksey Yeschenko (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CASSANDRA-4875?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Aleksey Yeschenko updated CASSANDRA-4875:
-----------------------------------------

    Fix Version/s: 1.2.0 rc1
    
> Possible improvements to IAuthority[2] interface
> ------------------------------------------------
>
>                 Key: CASSANDRA-4875
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4875
>             Project: Cassandra
>          Issue Type: Improvement
>    Affects Versions: 1.1.6, 1.2.0 beta 1
>            Reporter: Aleksey Yeschenko
>            Assignee: Aleksey Yeschenko
>              Labels: security
>             Fix For: 1.2.0 rc1
>
>
> CASSANDRA-4874 is about general improvements to authorization handling, this one is about IAuthority[2] in particular.
> - 'LIST GRANTS OF user should' become 'LIST PERMISSIONS [on resource] [of user]'.
> Currently there is no way to see all the permissions on the resource, only all the permissions of a particular user.
> - IAuthority2.listPermissions() should return a generic collection of ResoucePermission or something, not CQLResult or ResultMessage.
> That's a wrong level of abstraction. I know this issue has been raised here - https://issues.apache.org/jira/browse/CASSANDRA-4490?focusedCommentId=13449732&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732, but I think it's possible to change this. Returning a list of {resource, user, permission, grant_option} tuples should be possible.
> - We should get rid of Permission.NO_ACCESS. An empty list of permissions should mean absence of any permission, not some magical Permission.NO_ACCESS value.
> It's insecure and error-prone and also ambiguous (what if a user has both FULL_ACCESS and NO_ACCESS permissions? If it's meant to be a way to strip a user
> of all permissions on the resource, then it should be replaced with some form of REVOKE statement. Something like 'REVOKE ALL PERMISSIONS' sounds more logical than GRANT NO_ACCESS to me.
> - Previous point will probably require adding revokeAllPermissions() method to make it explicit, special-casing IAuthority2.revoke() won't do
> - IAuthorize2.grant() and IAuthorize2.revoke() accept CFName instance for a resource, which has its ks and cf fields swapped if cf is omitted. This may cause a real security issue if IAuthorize2 implementer doesn't know about the issue. We must pass the resouce as a collection of strings ([cassandra, keyspaces[, ks_name][, cf_name]]) instead, the way we pass it to IAuthorize.authorize().
> - We should probably get rid of FULL_ACCESS as well, at least as a valid permission value (but maybe allow it in the CQL statement) and add an equivalent IAuthority2.grantAllPermissions(), separately. Why? Imagine the following sequence: GRANT FULL_ACCESS ON resource FOR user; REVOKE SELECT ON resource FROM user; should the user be allowed to SELECT anymore?
> I say no, he shouldn't. Full access should be represented by a list of all permissions, not by a magical special value.
> - P.DELETE probably should go in favour of P.UPDATE even for TRUNCATE. Presence of P.DELETE will definitely confuse users, who might think that it is somehow required to delete data, when it isn't. You can overwrite every value if you have P.UPDATE with TTL=1 and get the same result. We should also drop P.INSERT. Leave P.UPDATE (or rename it to P.MODIFY). P.MODIFY_DATA + P.READ_DATA should replace P.UPDATE, P.SELECT and P.DELETE.
> - I suggest new syntax to allow setting permissions on cassandra/keyspaces resource: GRANT <permission> ON * FOR <user>.
> The interface has to change because of the CFName argument to grant() and revoke(), and since it's going to be broken anyway (and has been introduced recently), I think we are in a position to make some other improvements while at it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4875) Possible improvements to IAuthority[2] interface

Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13489065#comment-13489065 ] 

Jonathan Ellis commented on CASSANDRA-4875:
-------------------------------------------

bq. What about requiring the full name of the resource

Yes.

bq. getting rid of P.FULL_ACCESS

Yes.

bq. 'LIST PERMISSIONS [of user] [on resource]'

I misunderstood what you were saying, it's a yes.
                
> Possible improvements to IAuthority[2] interface
> ------------------------------------------------
>
>                 Key: CASSANDRA-4875
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4875
>             Project: Cassandra
>          Issue Type: Improvement
>    Affects Versions: 1.1.6, 1.2.0 beta 1
>            Reporter: Aleksey Yeschenko
>            Assignee: Aleksey Yeschenko
>              Labels: security
>
> CASSANDRA-4874 is about general improvements to authorization handling, this one is about IAuthority[2] in particular.
> - 'LIST GRANTS OF user should' become 'LIST PERMISSIONS [on resource] [of user]'.
> Currently there is no way to see all the permissions on the resource, only all the permissions of a particular user.
> - IAuthority2.listPermissions() should return a generic collection of ResoucePermission or something, not CQLResult or ResultMessage.
> That's a wrong level of abstraction. I know this issue has been raised here - https://issues.apache.org/jira/browse/CASSANDRA-4490?focusedCommentId=13449732&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732, but I think it's possible to change this. Returning a list of {resource, user, permission, grant_option} tuples should be possible.
> - We should get rid of Permission.NO_ACCESS. An empty list of permissions should mean absence of any permission, not some magical Permission.NO_ACCESS value.
> It's insecure and error-prone and also ambiguous (what if a user has both FULL_ACCESS and NO_ACCESS permissions? If it's meant to be a way to strip a user
> of all permissions on the resource, then it should be replaced with some form of REVOKE statement. Something like 'REVOKE ALL PERMISSIONS' sounds more logical than GRANT NO_ACCESS to me.
> - Previous point will probably require adding revokeAllPermissions() method to make it explicit, special-casing IAuthority2.revoke() won't do
> - IAuthorize2.grant() and IAuthorize2.revoke() accept CFName instance for a resource, which has its ks and cf fields swapped if cf is omitted. This may cause a real security issue if IAuthorize2 implementer doesn't know about the issue. We must pass the resouce as a collection of strings ([cassandra, keyspaces[, ks_name][, cf_name]]) instead, the way we pass it to IAuthorize.authorize().
> - We should probably get rid of FULL_ACCESS as well, at least as a valid permission value (but maybe allow it in the CQL statement) and add an equivalent IAuthority2.grantAllPermissions(), separately. Why? Imagine the following sequence: GRANT FULL_ACCESS ON resource FOR user; REVOKE SELECT ON resource FROM user; should the user be allowed to SELECT anymore?
> I say no, he shouldn't. Full access should be represented by a list of all permissions, not by a magical special value.
> - P.DELETE probably should go in favour of P.UPDATE even for TRUNCATE. Presence of P.DELETE will definitely confuse users, who might think that it is somehow required to delete data, when it isn't. You can overwrite every value if you have P.UPDATE with TTL=1 and get the same result. We should also drop P.INSERT. Leave P.UPDATE (or rename it to P.MODIFY). P.MODIFY_DATA + P.READ_DATA should replace P.UPDATE, P.SELECT and P.DELETE.
> - I suggest new syntax to allow setting permissions on cassandra/keyspaces resource: GRANT <permission> ON * FOR <user>.
> The interface has to change because of the CFName argument to grant() and revoke(), and since it's going to be broken anyway (and has been introduced recently), I think we are in a position to make some other improvements while at it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4875) Possible improvements to IAuthority[2] interface

Posted by "Aleksey Yeschenko (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13488866#comment-13488866 ] 

Aleksey Yeschenko commented on CASSANDRA-4875:
----------------------------------------------

bq. I don't think FULL_ACCESS is problematic if hierarchy traversal is done correctly. In your example, select would indeed be disallowed.

This is the problem - it wouldn't be disallowed since FULL_ACCESS is a permission on its own. If you grant FULL_ACCESS, then REVOKE SELECT - you still have your FULL_ACCESS. On the other hand, if GRANT ALL works as 'for permission in listPermissions(): grant(permission)', then GRANT ALL followed by REVOKE SELECT will leave the user with all the permissions except SELECT, which is exactly how we want this whole thing to behave.
                
> Possible improvements to IAuthority[2] interface
> ------------------------------------------------
>
>                 Key: CASSANDRA-4875
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4875
>             Project: Cassandra
>          Issue Type: Improvement
>    Affects Versions: 1.1.6, 1.2.0 beta 1
>            Reporter: Aleksey Yeschenko
>            Assignee: Aleksey Yeschenko
>              Labels: security
>
> CASSANDRA-4874 is about general improvements to authorization handling, this one is about IAuthority[2] in particular.
> - 'LIST GRANTS OF user should' become 'LIST PERMISSIONS [on resource] [of user]'.
> Currently there is no way to see all the permissions on the resource, only all the permissions of a particular user.
> - IAuthority2.listPermissions() should return a generic collection of ResoucePermission or something, not CQLResult or ResultMessage.
> That's a wrong level of abstraction. I know this issue has been raised here - https://issues.apache.org/jira/browse/CASSANDRA-4490?focusedCommentId=13449732&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732, but I think it's possible to change this. Returning a list of {resource, user, permission, grant_option} tuples should be possible.
> - We should get rid of Permission.NO_ACCESS. An empty list of permissions should mean absence of any permission, not some magical Permission.NO_ACCESS value.
> It's insecure and error-prone and also ambiguous (what if a user has both FULL_ACCESS and NO_ACCESS permissions? If it's meant to be a way to strip a user
> of all permissions on the resource, then it should be replaced with some form of REVOKE statement. Something like 'REVOKE ALL PERMISSIONS' sounds more logical than GRANT NO_ACCESS to me.
> - Previous point will probably require adding revokeAllPermissions() method to make it explicit, special-casing IAuthority2.revoke() won't do
> - IAuthorize2.grant() and IAuthorize2.revoke() accept CFName instance for a resource, which has its ks and cf fields swapped if cf is omitted. This may cause a real security issue if IAuthorize2 implementer doesn't know about the issue. We must pass the resouce as a collection of strings ([cassandra, keyspaces[, ks_name][, cf_name]]) instead, the way we pass it to IAuthorize.authorize().
> - We should probably get rid of FULL_ACCESS as well, at least as a valid permission value (but maybe allow it in the CQL statement) and add an equivalent IAuthority2.grantAllPermissions(), separately. Why? Imagine the following sequence: GRANT FULL_ACCESS ON resource FOR user; REVOKE SELECT ON resource FROM user; should the user be allowed to SELECT anymore?
> I say no, he shouldn't. Full access should be represented by a list of all permissions, not by a magical special value.
> - P.DELETE probably should go in favour of P.UPDATE even for TRUNCATE. Presence of P.DELETE will definitely confuse users, who might think that it is somehow required to delete data, when it isn't. You can overwrite every value if you have P.UPDATE with TTL=1 and get the same result. We should also drop P.INSERT. Leave P.UPDATE (or rename it to P.MODIFY). P.MODIFY_DATA + P.READ_DATA should replace P.UPDATE, P.SELECT and P.DELETE.
> - I suggest new syntax to allow setting permissions on cassandra/keyspaces resource: GRANT <permission> ON * FOR <user>.
> The interface has to change because of the CFName argument to grant() and revoke(), and since it's going to be broken anyway (and has been introduced recently), I think we are in a position to make some other improvements while at it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4875) Possible improvements to IAuthority[2] interface

Posted by "Aleksey Yeschenko (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13488173#comment-13488173 ] 

Aleksey Yeschenko commented on CASSANDRA-4875:
----------------------------------------------

Re: 'P.DELETE should probably go in favour of P.UPDATE' and TTL: we should just require both P.DELETE and P.UPDATE on inserts/updates with TTL set (discussed in CASSANDRA-4874)
                
> Possible improvements to IAuthority[2] interface
> ------------------------------------------------
>
>                 Key: CASSANDRA-4875
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4875
>             Project: Cassandra
>          Issue Type: Improvement
>    Affects Versions: 1.1.6, 1.2.0 beta 1
>            Reporter: Aleksey Yeschenko
>            Assignee: Aleksey Yeschenko
>              Labels: security
>
> CASSANDRA-4874 is about general improvements to authorization handling, this one is about IAuthority[2] in particular.
> - 'LIST GRANTS OF user should' become 'LIST PERMISSIONS [on resource] [of user]'.
> Currently there is no way to see all the permissions on the resource, only all the permissions of a particular user.
> - IAuthority2.listPermissions() should return a generic collection of ResoucePermission or something, not CQLResult or ResultMessage.
> That's a wrong level of abstraction. I know this issue has been raised here - https://issues.apache.org/jira/browse/CASSANDRA-4490?focusedCommentId=13449732&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732, but I think it's possible to change this. Returning a list of {resource, user, permission, grant_option} tuples should be possible.
> - We should get rid of Permission.NO_ACCESS. An empty list of permissions should mean absence of any permission, not some magical Permission.NO_ACCESS value.
> It's insecure and error-prone and also ambiguous (what if a user has both FULL_ACCESS and NO_ACCESS permissions? If it's meant to be a way to strip a user
> of all permissions on the resource, then it should be replaced with some form of REVOKE statement. Something like 'REVOKE ALL PERMISSIONS' sounds more logical than GRANT NO_ACCESS to me.
> - Previous point will probably require adding revokeAllPermissions() method to make it explicit, special-casing IAuthority2.revoke() won't do
> - IAuthorize2.grant() and IAuthorize2.revoke() accept CFName instance for a resource, which has its ks and cf fields swapped if cf is omitted. This may cause a real security issue if IAuthorize2 implementer doesn't know about the issue. We must pass the resouce as a collection of strings ([cassandra, keyspaces[, ks_name][, cf_name]]) instead, the way we pass it to IAuthorize.authorize().
> - We should probably get rid of FULL_ACCESS as well, at least as a valid permission value (but maybe allow it in the CQL statement) and add an equivalent IAuthority2.grantAllPermissions(), separately. Why? Imagine the following sequence: GRANT FULL_ACCESS ON resource FOR user; REVOKE SELECT ON resource FROM user; should the user be allowed to SELECT anymore?
> I say no, he shouldn't. Full access should be represented by a list of all permissions, not by a magical special value.
> - P.DELETE probably should go in favour of P.UPDATE even for TRUNCATE. Presence of P.DELETE will definitely confuse users, who might think that it is somehow required to delete data, when it isn't. You can overwrite every value if you have P.UPDATE with TTL=1 and get the same result. We should also drop P.INSERT. Leave P.UPDATE (or rename it to P.MODIFY). P.MODIFY_DATA + P.READ_DATA should replace P.UPDATE, P.SELECT and P.DELETE.
> - I suggest new syntax to allow setting permissions on cassandra/keyspaces resource: GRANT <permission> ON * FOR <user>.
> The interface has to change because of the CFName argument to grant() and revoke(), and since it's going to be broken anyway (and has been introduced recently), I think we are in a position to make some other improvements while at it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4875) Possible improvements to IAuthority[2] interface

Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13488958#comment-13488958 ] 

Jonathan Ellis commented on CASSANDRA-4875:
-------------------------------------------

I think we covered everything?
                
> Possible improvements to IAuthority[2] interface
> ------------------------------------------------
>
>                 Key: CASSANDRA-4875
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4875
>             Project: Cassandra
>          Issue Type: Improvement
>    Affects Versions: 1.1.6, 1.2.0 beta 1
>            Reporter: Aleksey Yeschenko
>            Assignee: Aleksey Yeschenko
>              Labels: security
>
> CASSANDRA-4874 is about general improvements to authorization handling, this one is about IAuthority[2] in particular.
> - 'LIST GRANTS OF user should' become 'LIST PERMISSIONS [on resource] [of user]'.
> Currently there is no way to see all the permissions on the resource, only all the permissions of a particular user.
> - IAuthority2.listPermissions() should return a generic collection of ResoucePermission or something, not CQLResult or ResultMessage.
> That's a wrong level of abstraction. I know this issue has been raised here - https://issues.apache.org/jira/browse/CASSANDRA-4490?focusedCommentId=13449732&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732, but I think it's possible to change this. Returning a list of {resource, user, permission, grant_option} tuples should be possible.
> - We should get rid of Permission.NO_ACCESS. An empty list of permissions should mean absence of any permission, not some magical Permission.NO_ACCESS value.
> It's insecure and error-prone and also ambiguous (what if a user has both FULL_ACCESS and NO_ACCESS permissions? If it's meant to be a way to strip a user
> of all permissions on the resource, then it should be replaced with some form of REVOKE statement. Something like 'REVOKE ALL PERMISSIONS' sounds more logical than GRANT NO_ACCESS to me.
> - Previous point will probably require adding revokeAllPermissions() method to make it explicit, special-casing IAuthority2.revoke() won't do
> - IAuthorize2.grant() and IAuthorize2.revoke() accept CFName instance for a resource, which has its ks and cf fields swapped if cf is omitted. This may cause a real security issue if IAuthorize2 implementer doesn't know about the issue. We must pass the resouce as a collection of strings ([cassandra, keyspaces[, ks_name][, cf_name]]) instead, the way we pass it to IAuthorize.authorize().
> - We should probably get rid of FULL_ACCESS as well, at least as a valid permission value (but maybe allow it in the CQL statement) and add an equivalent IAuthority2.grantAllPermissions(), separately. Why? Imagine the following sequence: GRANT FULL_ACCESS ON resource FOR user; REVOKE SELECT ON resource FROM user; should the user be allowed to SELECT anymore?
> I say no, he shouldn't. Full access should be represented by a list of all permissions, not by a magical special value.
> - P.DELETE probably should go in favour of P.UPDATE even for TRUNCATE. Presence of P.DELETE will definitely confuse users, who might think that it is somehow required to delete data, when it isn't. You can overwrite every value if you have P.UPDATE with TTL=1 and get the same result. We should also drop P.INSERT. Leave P.UPDATE (or rename it to P.MODIFY). P.MODIFY_DATA + P.READ_DATA should replace P.UPDATE, P.SELECT and P.DELETE.
> - I suggest new syntax to allow setting permissions on cassandra/keyspaces resource: GRANT <permission> ON * FOR <user>.
> The interface has to change because of the CFName argument to grant() and revoke(), and since it's going to be broken anyway (and has been introduced recently), I think we are in a position to make some other improvements while at it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4875) Possible improvements to IAuthority[2] interface

Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13488823#comment-13488823 ] 

Jonathan Ellis commented on CASSANDRA-4875:
-------------------------------------------

- 'LIST PERMISSIONS [of user] [on resource]' is slightly more grammatical
- Agreed that IAuthority2.listPermissions() should return a generic collection (Set?) of ResourcePermission
- +1 on removing NO-ACCESS
- We do want "REVOKE ALL" but "for permission in listPermissions(): revoke(permission)" seems adequate to implement that
- Agreed that CFName is not a good fit for "a keyspace or a columnfamily."  Not a huge fan of the List<String> approach either, though.  What about adding an IPermissionable interface that could be either a KS or a CF?  That would also allow adding other types of permissionable objects down the road -- functions is a likely candidate.
- I don't think FULL_ACCESS is problematic if hierarchy traversal is done correctly.  In your example, select would indeed be disallowed.  The problem with requiring permissions to be enumerated is, you add a new CF and now none of your GRANT ALL users have access to it unless you spell it out for each.  Painful.  (And not the way other databases work.)
- Parenthetically, FULL_ACCESS should really be ALL as far as cql is concerned.  Internally it doesn't matter.
- I'm okay with MODIFY + SELECT.  (Don't see any compelling reason to rename SELECT to READ.)
- I don't think we need new syntax?

Finally,
- Committing IAuth2 to 1.1.6 was a mistake.  Should we rip it out in 1.1.7?  If we do not, how sure are we that we won't have changes for 1.1.8?
                
> Possible improvements to IAuthority[2] interface
> ------------------------------------------------
>
>                 Key: CASSANDRA-4875
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4875
>             Project: Cassandra
>          Issue Type: Improvement
>    Affects Versions: 1.1.6, 1.2.0 beta 1
>            Reporter: Aleksey Yeschenko
>            Assignee: Aleksey Yeschenko
>              Labels: security
>
> CASSANDRA-4874 is about general improvements to authorization handling, this one is about IAuthority[2] in particular.
> - 'LIST GRANTS OF user should' become 'LIST PERMISSIONS [on resource] [of user]'.
> Currently there is no way to see all the permissions on the resource, only all the permissions of a particular user.
> - IAuthority2.listPermissions() should return a generic collection of ResoucePermission or something, not CQLResult or ResultMessage.
> That's a wrong level of abstraction. I know this issue has been raised here - https://issues.apache.org/jira/browse/CASSANDRA-4490?focusedCommentId=13449732&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732, but I think it's possible to change this. Returning a list of {resource, user, permission, grant_option} tuples should be possible.
> - We should get rid of Permission.NO_ACCESS. An empty list of permissions should mean absence of any permission, not some magical Permission.NO_ACCESS value.
> It's insecure and error-prone and also ambiguous (what if a user has both FULL_ACCESS and NO_ACCESS permissions? If it's meant to be a way to strip a user
> of all permissions on the resource, then it should be replaced with some form of REVOKE statement. Something like 'REVOKE ALL PERMISSIONS' sounds more logical than GRANT NO_ACCESS to me.
> - Previous point will probably require adding revokeAllPermissions() method to make it explicit, special-casing IAuthority2.revoke() won't do
> - IAuthorize2.grant() and IAuthorize2.revoke() accept CFName instance for a resource, which has its ks and cf fields swapped if cf is omitted. This may cause a real security issue if IAuthorize2 implementer doesn't know about the issue. We must pass the resouce as a collection of strings ([cassandra, keyspaces[, ks_name][, cf_name]]) instead, the way we pass it to IAuthorize.authorize().
> - We should probably get rid of FULL_ACCESS as well, at least as a valid permission value (but maybe allow it in the CQL statement) and add an equivalent IAuthority2.grantAllPermissions(), separately. Why? Imagine the following sequence: GRANT FULL_ACCESS ON resource FOR user; REVOKE SELECT ON resource FROM user; should the user be allowed to SELECT anymore?
> I say no, he shouldn't. Full access should be represented by a list of all permissions, not by a magical special value.
> - P.DELETE probably should go in favour of P.UPDATE even for TRUNCATE. Presence of P.DELETE will definitely confuse users, who might think that it is somehow required to delete data, when it isn't. You can overwrite every value if you have P.UPDATE with TTL=1 and get the same result. We should also drop P.INSERT. Leave P.UPDATE (or rename it to P.MODIFY). P.MODIFY_DATA + P.READ_DATA should replace P.UPDATE, P.SELECT and P.DELETE.
> - I suggest new syntax to allow setting permissions on cassandra/keyspaces resource: GRANT <permission> ON * FOR <user>.
> The interface has to change because of the CFName argument to grant() and revoke(), and since it's going to be broken anyway (and has been introduced recently), I think we are in a position to make some other improvements while at it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4875) Possible improvements to IAuthority[2] interface

Posted by "Aleksey Yeschenko (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13488952#comment-13488952 ] 

Aleksey Yeschenko commented on CASSANDRA-4875:
----------------------------------------------

bq. No, would need a new method and translate the old one.
Right.

Any thoughts on other points?
                
> Possible improvements to IAuthority[2] interface
> ------------------------------------------------
>
>                 Key: CASSANDRA-4875
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4875
>             Project: Cassandra
>          Issue Type: Improvement
>    Affects Versions: 1.1.6, 1.2.0 beta 1
>            Reporter: Aleksey Yeschenko
>            Assignee: Aleksey Yeschenko
>              Labels: security
>
> CASSANDRA-4874 is about general improvements to authorization handling, this one is about IAuthority[2] in particular.
> - 'LIST GRANTS OF user should' become 'LIST PERMISSIONS [on resource] [of user]'.
> Currently there is no way to see all the permissions on the resource, only all the permissions of a particular user.
> - IAuthority2.listPermissions() should return a generic collection of ResoucePermission or something, not CQLResult or ResultMessage.
> That's a wrong level of abstraction. I know this issue has been raised here - https://issues.apache.org/jira/browse/CASSANDRA-4490?focusedCommentId=13449732&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732, but I think it's possible to change this. Returning a list of {resource, user, permission, grant_option} tuples should be possible.
> - We should get rid of Permission.NO_ACCESS. An empty list of permissions should mean absence of any permission, not some magical Permission.NO_ACCESS value.
> It's insecure and error-prone and also ambiguous (what if a user has both FULL_ACCESS and NO_ACCESS permissions? If it's meant to be a way to strip a user
> of all permissions on the resource, then it should be replaced with some form of REVOKE statement. Something like 'REVOKE ALL PERMISSIONS' sounds more logical than GRANT NO_ACCESS to me.
> - Previous point will probably require adding revokeAllPermissions() method to make it explicit, special-casing IAuthority2.revoke() won't do
> - IAuthorize2.grant() and IAuthorize2.revoke() accept CFName instance for a resource, which has its ks and cf fields swapped if cf is omitted. This may cause a real security issue if IAuthorize2 implementer doesn't know about the issue. We must pass the resouce as a collection of strings ([cassandra, keyspaces[, ks_name][, cf_name]]) instead, the way we pass it to IAuthorize.authorize().
> - We should probably get rid of FULL_ACCESS as well, at least as a valid permission value (but maybe allow it in the CQL statement) and add an equivalent IAuthority2.grantAllPermissions(), separately. Why? Imagine the following sequence: GRANT FULL_ACCESS ON resource FOR user; REVOKE SELECT ON resource FROM user; should the user be allowed to SELECT anymore?
> I say no, he shouldn't. Full access should be represented by a list of all permissions, not by a magical special value.
> - P.DELETE probably should go in favour of P.UPDATE even for TRUNCATE. Presence of P.DELETE will definitely confuse users, who might think that it is somehow required to delete data, when it isn't. You can overwrite every value if you have P.UPDATE with TTL=1 and get the same result. We should also drop P.INSERT. Leave P.UPDATE (or rename it to P.MODIFY). P.MODIFY_DATA + P.READ_DATA should replace P.UPDATE, P.SELECT and P.DELETE.
> - I suggest new syntax to allow setting permissions on cassandra/keyspaces resource: GRANT <permission> ON * FOR <user>.
> The interface has to change because of the CFName argument to grant() and revoke(), and since it's going to be broken anyway (and has been introduced recently), I think we are in a position to make some other improvements while at it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (CASSANDRA-4875) Revert IAuthority2 interface

Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CASSANDRA-4875?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jonathan Ellis updated CASSANDRA-4875:
--------------------------------------

    Affects Version/s:     (was: 1.2.0 beta 1)
        Fix Version/s:     (was: 1.2.0 rc1)
                       1.1.7
              Summary: Revert IAuthority2 interface  (was: Possible improvements to IAuthority[2] interface)
    
> Revert IAuthority2 interface
> ----------------------------
>
>                 Key: CASSANDRA-4875
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4875
>             Project: Cassandra
>          Issue Type: Improvement
>    Affects Versions: 1.1.6
>            Reporter: Aleksey Yeschenko
>            Assignee: Aleksey Yeschenko
>              Labels: security
>             Fix For: 1.1.7
>
>         Attachments: 0001-Rip-out-IAuthority2-from-1.1.txt
>
>
> CASSANDRA-4874 is about general improvements to authorization handling, this one is about IAuthority[2] in particular.
> - 'LIST GRANTS OF user should' become 'LIST PERMISSIONS [on resource] [of user]'.
> Currently there is no way to see all the permissions on the resource, only all the permissions of a particular user.
> - IAuthority2.listPermissions() should return a generic collection of ResoucePermission or something, not CQLResult or ResultMessage.
> That's a wrong level of abstraction. I know this issue has been raised here - https://issues.apache.org/jira/browse/CASSANDRA-4490?focusedCommentId=13449732&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732, but I think it's possible to change this. Returning a list of {resource, user, permission, grant_option} tuples should be possible.
> - We should get rid of Permission.NO_ACCESS. An empty list of permissions should mean absence of any permission, not some magical Permission.NO_ACCESS value.
> It's insecure and error-prone and also ambiguous (what if a user has both FULL_ACCESS and NO_ACCESS permissions? If it's meant to be a way to strip a user
> of all permissions on the resource, then it should be replaced with some form of REVOKE statement. Something like 'REVOKE ALL PERMISSIONS' sounds more logical than GRANT NO_ACCESS to me.
> - Previous point will probably require adding revokeAllPermissions() method to make it explicit, special-casing IAuthority2.revoke() won't do
> - IAuthorize2.grant() and IAuthorize2.revoke() accept CFName instance for a resource, which has its ks and cf fields swapped if cf is omitted. This may cause a real security issue if IAuthorize2 implementer doesn't know about the issue. We must pass the resouce as a collection of strings ([cassandra, keyspaces[, ks_name][, cf_name]]) instead, the way we pass it to IAuthorize.authorize().
> - We should probably get rid of FULL_ACCESS as well, at least as a valid permission value (but maybe allow it in the CQL statement) and add an equivalent IAuthority2.grantAllPermissions(), separately. Why? Imagine the following sequence: GRANT FULL_ACCESS ON resource FOR user; REVOKE SELECT ON resource FROM user; should the user be allowed to SELECT anymore?
> I say no, he shouldn't. Full access should be represented by a list of all permissions, not by a magical special value.
> - P.DELETE probably should go in favour of P.UPDATE even for TRUNCATE. Presence of P.DELETE will definitely confuse users, who might think that it is somehow required to delete data, when it isn't. You can overwrite every value if you have P.UPDATE with TTL=1 and get the same result. We should also drop P.INSERT. Leave P.UPDATE (or rename it to P.MODIFY). P.MODIFY_DATA + P.READ_DATA should replace P.UPDATE, P.SELECT and P.DELETE.
> - I suggest new syntax to allow setting permissions on cassandra/keyspaces resource: GRANT <permission> ON * FOR <user>.
> The interface has to change because of the CFName argument to grant() and revoke(), and since it's going to be broken anyway (and has been introduced recently), I think we are in a position to make some other improvements while at it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4875) Possible improvements to IAuthority[2] interface

Posted by "Aleksey Yeschenko (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13488848#comment-13488848 ] 

Aleksey Yeschenko commented on CASSANDRA-4875:
----------------------------------------------

bq. 'LIST PERMISSIONS [of user] [on resource]' is slightly more grammatical
Is it a good thing or a bad thing? In any case, being able to list all the permissions on the resource is useful. It's currently impossible to know who has access to a particular resource - you need to somehow get the full list of users and then for user in all_users: list grants for user.

bq. We do want "REVOKE ALL" but "for permission in listPermissions(): revoke(permission)" seems adequate to implement that.
I see now. And agree.

bq. What about adding an IPermissionable interface that could be either a KS or a CF?
Maybe (most likely) a list of strings is not a good idea, but something capable of representing the complete hierarchy is needed.
Currently it's cassandra/keyspaces[/ks[/cf]], and we don't check or have a way to set permissions on the first two levels. I think we should drop 'cassandra'. Make it keyspaces[/ks[/cf]] or data[/ks[/cf]] or something like that and require specifying the whole path when granting/revoking.
'GRANT CREATE ON keyspaces' would allow creating keyspaces without explicit permission on the not-yet-existing ks, for example (the issue from 4874). 'GRANT CREATE ON keyspaces/test', 'GRANT MODIFY ON keyspaces/test/cf'. This is slightly more verbose, but also more flexible - if we add other types of permissionable objects, as you said, we won't have to change grant/revoke syntax.

                
> Possible improvements to IAuthority[2] interface
> ------------------------------------------------
>
>                 Key: CASSANDRA-4875
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4875
>             Project: Cassandra
>          Issue Type: Improvement
>    Affects Versions: 1.1.6, 1.2.0 beta 1
>            Reporter: Aleksey Yeschenko
>            Assignee: Aleksey Yeschenko
>              Labels: security
>
> CASSANDRA-4874 is about general improvements to authorization handling, this one is about IAuthority[2] in particular.
> - 'LIST GRANTS OF user should' become 'LIST PERMISSIONS [on resource] [of user]'.
> Currently there is no way to see all the permissions on the resource, only all the permissions of a particular user.
> - IAuthority2.listPermissions() should return a generic collection of ResoucePermission or something, not CQLResult or ResultMessage.
> That's a wrong level of abstraction. I know this issue has been raised here - https://issues.apache.org/jira/browse/CASSANDRA-4490?focusedCommentId=13449732&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732, but I think it's possible to change this. Returning a list of {resource, user, permission, grant_option} tuples should be possible.
> - We should get rid of Permission.NO_ACCESS. An empty list of permissions should mean absence of any permission, not some magical Permission.NO_ACCESS value.
> It's insecure and error-prone and also ambiguous (what if a user has both FULL_ACCESS and NO_ACCESS permissions? If it's meant to be a way to strip a user
> of all permissions on the resource, then it should be replaced with some form of REVOKE statement. Something like 'REVOKE ALL PERMISSIONS' sounds more logical than GRANT NO_ACCESS to me.
> - Previous point will probably require adding revokeAllPermissions() method to make it explicit, special-casing IAuthority2.revoke() won't do
> - IAuthorize2.grant() and IAuthorize2.revoke() accept CFName instance for a resource, which has its ks and cf fields swapped if cf is omitted. This may cause a real security issue if IAuthorize2 implementer doesn't know about the issue. We must pass the resouce as a collection of strings ([cassandra, keyspaces[, ks_name][, cf_name]]) instead, the way we pass it to IAuthorize.authorize().
> - We should probably get rid of FULL_ACCESS as well, at least as a valid permission value (but maybe allow it in the CQL statement) and add an equivalent IAuthority2.grantAllPermissions(), separately. Why? Imagine the following sequence: GRANT FULL_ACCESS ON resource FOR user; REVOKE SELECT ON resource FROM user; should the user be allowed to SELECT anymore?
> I say no, he shouldn't. Full access should be represented by a list of all permissions, not by a magical special value.
> - P.DELETE probably should go in favour of P.UPDATE even for TRUNCATE. Presence of P.DELETE will definitely confuse users, who might think that it is somehow required to delete data, when it isn't. You can overwrite every value if you have P.UPDATE with TTL=1 and get the same result. We should also drop P.INSERT. Leave P.UPDATE (or rename it to P.MODIFY). P.MODIFY_DATA + P.READ_DATA should replace P.UPDATE, P.SELECT and P.DELETE.
> - I suggest new syntax to allow setting permissions on cassandra/keyspaces resource: GRANT <permission> ON * FOR <user>.
> The interface has to change because of the CFName argument to grant() and revoke(), and since it's going to be broken anyway (and has been introduced recently), I think we are in a position to make some other improvements while at it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4875) Possible improvements to IAuthority[2] interface

Posted by "Aleksey Yeschenko (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13488872#comment-13488872 ] 

Aleksey Yeschenko commented on CASSANDRA-4875:
----------------------------------------------

Back to representing the resource.. a list of strings is what IAuthority.authorize() expects. I think it's fine to break IAuthority2 since it's broken anyway, but can we change IAuthority as well?
                
> Possible improvements to IAuthority[2] interface
> ------------------------------------------------
>
>                 Key: CASSANDRA-4875
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4875
>             Project: Cassandra
>          Issue Type: Improvement
>    Affects Versions: 1.1.6, 1.2.0 beta 1
>            Reporter: Aleksey Yeschenko
>            Assignee: Aleksey Yeschenko
>              Labels: security
>
> CASSANDRA-4874 is about general improvements to authorization handling, this one is about IAuthority[2] in particular.
> - 'LIST GRANTS OF user should' become 'LIST PERMISSIONS [on resource] [of user]'.
> Currently there is no way to see all the permissions on the resource, only all the permissions of a particular user.
> - IAuthority2.listPermissions() should return a generic collection of ResoucePermission or something, not CQLResult or ResultMessage.
> That's a wrong level of abstraction. I know this issue has been raised here - https://issues.apache.org/jira/browse/CASSANDRA-4490?focusedCommentId=13449732&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732, but I think it's possible to change this. Returning a list of {resource, user, permission, grant_option} tuples should be possible.
> - We should get rid of Permission.NO_ACCESS. An empty list of permissions should mean absence of any permission, not some magical Permission.NO_ACCESS value.
> It's insecure and error-prone and also ambiguous (what if a user has both FULL_ACCESS and NO_ACCESS permissions? If it's meant to be a way to strip a user
> of all permissions on the resource, then it should be replaced with some form of REVOKE statement. Something like 'REVOKE ALL PERMISSIONS' sounds more logical than GRANT NO_ACCESS to me.
> - Previous point will probably require adding revokeAllPermissions() method to make it explicit, special-casing IAuthority2.revoke() won't do
> - IAuthorize2.grant() and IAuthorize2.revoke() accept CFName instance for a resource, which has its ks and cf fields swapped if cf is omitted. This may cause a real security issue if IAuthorize2 implementer doesn't know about the issue. We must pass the resouce as a collection of strings ([cassandra, keyspaces[, ks_name][, cf_name]]) instead, the way we pass it to IAuthorize.authorize().
> - We should probably get rid of FULL_ACCESS as well, at least as a valid permission value (but maybe allow it in the CQL statement) and add an equivalent IAuthority2.grantAllPermissions(), separately. Why? Imagine the following sequence: GRANT FULL_ACCESS ON resource FOR user; REVOKE SELECT ON resource FROM user; should the user be allowed to SELECT anymore?
> I say no, he shouldn't. Full access should be represented by a list of all permissions, not by a magical special value.
> - P.DELETE probably should go in favour of P.UPDATE even for TRUNCATE. Presence of P.DELETE will definitely confuse users, who might think that it is somehow required to delete data, when it isn't. You can overwrite every value if you have P.UPDATE with TTL=1 and get the same result. We should also drop P.INSERT. Leave P.UPDATE (or rename it to P.MODIFY). P.MODIFY_DATA + P.READ_DATA should replace P.UPDATE, P.SELECT and P.DELETE.
> - I suggest new syntax to allow setting permissions on cassandra/keyspaces resource: GRANT <permission> ON * FOR <user>.
> The interface has to change because of the CFName argument to grant() and revoke(), and since it's going to be broken anyway (and has been introduced recently), I think we are in a position to make some other improvements while at it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4875) Possible improvements to IAuthority[2] interface

Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13488945#comment-13488945 ] 

Jonathan Ellis commented on CASSANDRA-4875:
-------------------------------------------

bq. Currently it's cassandra/keyspaces[/ks[/cf]], and we don't check or have a way to set permissions on the first two levels. I think we should drop 'cassandra'.

I'm not sure what the goal is here tbh -- i.e. if the "cassandra" prefix makes it easier for some external consumer.  I could see making it "clustername/[ks[/cf]]' for instance.

bq. If you create a new CF then your (ks level) GRANT ALL users will have complete access to it

+1

bq. a list of strings is what IAuthority.authorize() expects. I think it's fine to break IAuthority2 since it's broken anyway, but can we change IAuthority as well?

No, would need a new method and translate the old one.
                
> Possible improvements to IAuthority[2] interface
> ------------------------------------------------
>
>                 Key: CASSANDRA-4875
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4875
>             Project: Cassandra
>          Issue Type: Improvement
>    Affects Versions: 1.1.6, 1.2.0 beta 1
>            Reporter: Aleksey Yeschenko
>            Assignee: Aleksey Yeschenko
>              Labels: security
>
> CASSANDRA-4874 is about general improvements to authorization handling, this one is about IAuthority[2] in particular.
> - 'LIST GRANTS OF user should' become 'LIST PERMISSIONS [on resource] [of user]'.
> Currently there is no way to see all the permissions on the resource, only all the permissions of a particular user.
> - IAuthority2.listPermissions() should return a generic collection of ResoucePermission or something, not CQLResult or ResultMessage.
> That's a wrong level of abstraction. I know this issue has been raised here - https://issues.apache.org/jira/browse/CASSANDRA-4490?focusedCommentId=13449732&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732, but I think it's possible to change this. Returning a list of {resource, user, permission, grant_option} tuples should be possible.
> - We should get rid of Permission.NO_ACCESS. An empty list of permissions should mean absence of any permission, not some magical Permission.NO_ACCESS value.
> It's insecure and error-prone and also ambiguous (what if a user has both FULL_ACCESS and NO_ACCESS permissions? If it's meant to be a way to strip a user
> of all permissions on the resource, then it should be replaced with some form of REVOKE statement. Something like 'REVOKE ALL PERMISSIONS' sounds more logical than GRANT NO_ACCESS to me.
> - Previous point will probably require adding revokeAllPermissions() method to make it explicit, special-casing IAuthority2.revoke() won't do
> - IAuthorize2.grant() and IAuthorize2.revoke() accept CFName instance for a resource, which has its ks and cf fields swapped if cf is omitted. This may cause a real security issue if IAuthorize2 implementer doesn't know about the issue. We must pass the resouce as a collection of strings ([cassandra, keyspaces[, ks_name][, cf_name]]) instead, the way we pass it to IAuthorize.authorize().
> - We should probably get rid of FULL_ACCESS as well, at least as a valid permission value (but maybe allow it in the CQL statement) and add an equivalent IAuthority2.grantAllPermissions(), separately. Why? Imagine the following sequence: GRANT FULL_ACCESS ON resource FOR user; REVOKE SELECT ON resource FROM user; should the user be allowed to SELECT anymore?
> I say no, he shouldn't. Full access should be represented by a list of all permissions, not by a magical special value.
> - P.DELETE probably should go in favour of P.UPDATE even for TRUNCATE. Presence of P.DELETE will definitely confuse users, who might think that it is somehow required to delete data, when it isn't. You can overwrite every value if you have P.UPDATE with TTL=1 and get the same result. We should also drop P.INSERT. Leave P.UPDATE (or rename it to P.MODIFY). P.MODIFY_DATA + P.READ_DATA should replace P.UPDATE, P.SELECT and P.DELETE.
> - I suggest new syntax to allow setting permissions on cassandra/keyspaces resource: GRANT <permission> ON * FOR <user>.
> The interface has to change because of the CFName argument to grant() and revoke(), and since it's going to be broken anyway (and has been introduced recently), I think we are in a position to make some other improvements while at it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4875) Possible improvements to IAuthority[2] interface

Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13492491#comment-13492491 ] 

Jonathan Ellis commented on CASSANDRA-4875:
-------------------------------------------

committed patch to remove IA2 from 1.1
                
> Possible improvements to IAuthority[2] interface
> ------------------------------------------------
>
>                 Key: CASSANDRA-4875
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4875
>             Project: Cassandra
>          Issue Type: Improvement
>    Affects Versions: 1.1.6, 1.2.0 beta 1
>            Reporter: Aleksey Yeschenko
>            Assignee: Aleksey Yeschenko
>              Labels: security
>             Fix For: 1.2.0 rc1
>
>         Attachments: 0001-Rip-out-IAuthority2-from-1.1.txt
>
>
> CASSANDRA-4874 is about general improvements to authorization handling, this one is about IAuthority[2] in particular.
> - 'LIST GRANTS OF user should' become 'LIST PERMISSIONS [on resource] [of user]'.
> Currently there is no way to see all the permissions on the resource, only all the permissions of a particular user.
> - IAuthority2.listPermissions() should return a generic collection of ResoucePermission or something, not CQLResult or ResultMessage.
> That's a wrong level of abstraction. I know this issue has been raised here - https://issues.apache.org/jira/browse/CASSANDRA-4490?focusedCommentId=13449732&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732, but I think it's possible to change this. Returning a list of {resource, user, permission, grant_option} tuples should be possible.
> - We should get rid of Permission.NO_ACCESS. An empty list of permissions should mean absence of any permission, not some magical Permission.NO_ACCESS value.
> It's insecure and error-prone and also ambiguous (what if a user has both FULL_ACCESS and NO_ACCESS permissions? If it's meant to be a way to strip a user
> of all permissions on the resource, then it should be replaced with some form of REVOKE statement. Something like 'REVOKE ALL PERMISSIONS' sounds more logical than GRANT NO_ACCESS to me.
> - Previous point will probably require adding revokeAllPermissions() method to make it explicit, special-casing IAuthority2.revoke() won't do
> - IAuthorize2.grant() and IAuthorize2.revoke() accept CFName instance for a resource, which has its ks and cf fields swapped if cf is omitted. This may cause a real security issue if IAuthorize2 implementer doesn't know about the issue. We must pass the resouce as a collection of strings ([cassandra, keyspaces[, ks_name][, cf_name]]) instead, the way we pass it to IAuthorize.authorize().
> - We should probably get rid of FULL_ACCESS as well, at least as a valid permission value (but maybe allow it in the CQL statement) and add an equivalent IAuthority2.grantAllPermissions(), separately. Why? Imagine the following sequence: GRANT FULL_ACCESS ON resource FOR user; REVOKE SELECT ON resource FROM user; should the user be allowed to SELECT anymore?
> I say no, he shouldn't. Full access should be represented by a list of all permissions, not by a magical special value.
> - P.DELETE probably should go in favour of P.UPDATE even for TRUNCATE. Presence of P.DELETE will definitely confuse users, who might think that it is somehow required to delete data, when it isn't. You can overwrite every value if you have P.UPDATE with TTL=1 and get the same result. We should also drop P.INSERT. Leave P.UPDATE (or rename it to P.MODIFY). P.MODIFY_DATA + P.READ_DATA should replace P.UPDATE, P.SELECT and P.DELETE.
> - I suggest new syntax to allow setting permissions on cassandra/keyspaces resource: GRANT <permission> ON * FOR <user>.
> The interface has to change because of the CFName argument to grant() and revoke(), and since it's going to be broken anyway (and has been introduced recently), I think we are in a position to make some other improvements while at it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira