cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jonathan Ellis (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-4875) Possible improvements to IAuthority[2] interface
Date Thu, 01 Nov 2012 16:51:13 GMT

    [ 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

Mime
View raw message