cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sam Tunnicliffe (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-12859) Column-level permissions
Date Mon, 31 Oct 2016 12:20:58 GMT

    [ https://issues.apache.org/jira/browse/CASSANDRA-12859?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15622003#comment-15622003
] 

Sam Tunnicliffe commented on CASSANDRA-12859:
---------------------------------------------

Thanks for opening this and for the detailed proposal doc, I've heard this feature requested
a few times now, so it would be good to get it in. 
Regarding the specifics of the proposal, I have a few questions/points to feedback:

bq. In the interest of an unobtrusive and non-breaking implementation, I propose to not break
up MODIFY into its conceptual parts. Rather, optional column lists will be allowed on MODIFY.
Such column lists, if any, will be simply ignored in permission enforcement of DELETE and
TRUNCATE statements.

When you say column lists will be ignored, do you mean that the specific columns in the list
will be ignored, making the presence of *any* list equivalent to a table-level grant? Or is
the suggestion that a permission with a qualifying list of columns will be equivalent to *no*
permission being granted? The former is definitely dangerous as it would allow a user who
has MODIFY permission only on a single column to delete an entire row, or even to delete all
partitions with TRUNCATE. The latter is also somewhat problematic as it requires special handling
at authz time, i.e. you need to specifically check whether the user has non-column-restricted
permission on the table (which I think is subtly different to the checking required on the
read & upsert paths). Why not just process deletes/truncates the same as inserts?
----
bq. Dropping previously included columns from the new list has the effect of revoking the
permission on those columns.
If I understood this correctly, this means that every GRANT statement containing a column
list completely replaces any existing column list. e.g.
{code}
GRANT SELECT on ks.t1 (col_a, col_b) TO foo;  // role foo has access to col_a and col_b
GRANT SELECT on kt.t1 (col_c) TO foo;  // now foo only has permissions on col_c
{code}
The special case is when the column list is empty, in which case it becomes a GRANT on *all*
columns. I get that this special case is required for backwards compatibility, but I'm not
keen on the regular case as it seems a little counter-intuitive to me. After executing the
two statements above for example, it would appear more natural to me for foo to have SELECT
permissions on all three columns. 
----
bq. Are there unit tests, part of the Cassandra project, that verify functionality of managing
and enforcing permissions?
There are not any substantial unit tests for authz at the table level, but there is a fairly
comprehensive set of dtests [here|https://github.com/riptano/cassandra-dtest/blob/master/auth_test.py].
The main impediment to better unit testing here is that {{CassandraAuthorizer}} does all reads
and writes using the distributed path, through {{StorageProxy}}. I've been considering something
like [this change|https://gist.github.com/beobal/0bcd592ad7716d0bebd400c53b83ce3e] to make
it more testable, this might be a good time to do that. (Note: {{CassandraRoleManager}} works
in exactly the same way, so it will require similar changes to be used in unit tests).
----
How do you propose to handle dropped/altered columns? When a table or keyspace is dropped,
all permissions on it are revoked. Aside from good housekeeping, this prevents accidental
leakage of permissions, should a new table be created with the same name. {{IAuthorizer}}
is currently hooked up to schema change events via {{AuthMigrationListener}} to facilitate
this. Something similar will need to be done to process schema events which alter or drop
columns. This scenario is missing from the proposed testing plan btw.
----
Whilst the new EBNF looks fair enough, we need to be sure and enforce the restriction that
only {{DataResource}} can have a column list applied, and only a Table level {{DataResource}}
at that. So, although it's not something that can be enforced at the grammar level AFAICT,
we need to ensure that statements like these are illegal:
{code}
GRANT SELECT (col_a, col_b) ON KEYSPACE ks TO foo;
GRANT EXECUTE (col_x) ON FUNCTION ks.fun1(int) TO foo;
{code}
----
The section describing how the finer-grained checking will impact the code stops at the {{ClientState}}
& doesn't make any mention of changes to the {{IAuthorizer}} interface. So it's slightly
unclear how precisely to support 
bq. enriching the class PermissionsCache by managing, in memory, a set of included columns
(if specified in a GRANT statement), per SELECT / MODIFY. Needs to be looked up efficiently.
To be honest, I think this is a trickier problem than it may at first appear. The reason being
that the concept of qualifying permissions with a column list only applies to one specific
{{IResource}} implementation, but {{IAuthorizer::authorize}} is completely agnostic as to
type (and level) of the resource being accessed. So right now, I'm not entirely sure what
is the best way to proceed with that. It may be that a new column-level {{IResource}} might
be the cleanest way to go, but I haven't really thought through that yet. 


> Column-level permissions
> ------------------------
>
>                 Key: CASSANDRA-12859
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-12859
>             Project: Cassandra
>          Issue Type: New Feature
>          Components: Core, CQL
>            Reporter: Boris Melamed
>         Attachments: Cassandra Proposal - Column-level permissions.docx
>
>   Original Estimate: 504h
>  Remaining Estimate: 504h
>
> h4. Here is a draft of: 
> Cassandra Proposal - Column-level permissions.docx (attached)
> h4. Quoting the 'Overview' section:
> The purpose of this proposal is to add column-level (field-level) permissions to Cassandra.
It is my intent to soon start implementing this feature in a fork, and to submit a pull request
once it’s ready.
> h4. Motivation
> Cassandra already supports permissions on keyspace and table (column family) level. Sources:
> * http://www.datastax.com/dev/blog/role-based-access-control-in-cassandra
> * https://cassandra.apache.org/doc/latest/cql/security.html#data-control
> At IBM, we have use cases in the area of big data analytics where column-level access
permissions are also a requirement. All industry RDBMS products are supporting this level
of permission control, and regulators are expecting it from all data-based systems.
> h4. Main day-one requirements
> # Extend CQL (Cassandra Query Language) to be able to optionally specify a list of individual
columns, in the {{GRANT}} statement. The relevant permission types are: {{MODIFY}} (for {{UPDATE}}
and {{INSERT}}) and {{SELECT}}.
> # Persist the optional information in the appropriate system table ‘system_auth.role_permissions’.
> # Enforce the column access restrictions during execution. Details:
> #* Should fit with the existing permission propagation down a role chain.
> #* Proposed message format when a user’s roles give access to the queried table but
not to all of the selected, inserted, or updated columns:
>   "User %s has no %s permission on column %s of table %s"
> #* Error will report only the first checked column. 
> Nice to have: list all inaccessible columns.
> #* Error code is the same as for table access denial: 2100.
> h4. Additional day-one requirements
> # Reflect the column-level permissions in statements of type 
> {{LIST ALL PERMISSIONS OF someuser;}}
> # Performance should not degrade in any significant way.
> # Backwards compatibility
> #* Permission enforcement for DBs created before the upgrade should continue to work
with the same behavior after upgrading to a version that allows column-level permissions.
> #* Previous CQL syntax will remain valid, and have the same effect as before.
> h4. Documentation
> * https://cassandra.apache.org/doc/latest/cql/security.html#grammar-token-permission
> * Feedback request: any others?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message