[
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)
