Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12500
to look at the new patch set (#4).
Change subject: [sentry] add privilege scope validation to SentryAuthzProvider
......................................................................
[sentry] add privilege scope validation to SentryAuthzProvider
Currently, SentryAuthzProvider::Authorize() performs authorization base
on the following rules,
a privilege implies another when:
1. the authorizable from the former implies the authorizable from the
latter, and
2. the action from the former implies the action from the latter, and
3. grant option from the former implies the grant option from the latter.
We fetch from Sentry the privileges for the given user that might imply
the given authorizable and action (via
list_sentry_privileges_by_user_and_itsgroups
Sentry API). We then proceed to check the implication rules on the
returned actions. However, we rely on the API to enforce the implication
rules of the returned authorizables. This ignored the fact that the API
returns all privileges granted to the user that match the authorizable
of each scope in the input authorizable hierarchy (not just the implied
authorizables).
So when Alice tries to create table 'default.b' (this requires 'CREATE
ON DATABASE'), the Sentry API will return anything that matches:
server == "server1" && (db == "default" || db == NULL)
which may include 'ALL ON default.a'. Previously we would only take into
consideration the fact that 'ALL' is returned, and proceed to incorrectly
permit the create operation.
This patch adds privilege scope validation to SentryAuthzProvider to
ensure only authorizable with a higher scope on the hierarchy can imply
authorizables with a lower scope on the hierarchy.
Theoretically speaking, an alternative is to add authorizable scope filering
in Sentry server. However, list_sentry_privileges_by_user_and_itsgroups
API also needs to work with the default Sentry client policy validation
(org.apache.sentry.policy.common.CommonPrivilege), which supports wildcard
authorizable matching (e.g. authorizable 'server=server1->db=*' can imply
authorizable 'server=server1'). In such case, authorizable scope filering
is not appropriate. On the other hand, wildcard authorizable matching is
dropped in Kudu (see the comment in SentryAuthzProvider::Authorize()).
Change-Id: I89437a04a4fa18e501d21c3abf5d66a2d22ce58a
---
M src/kudu/master/sentry_authz_provider-test.cc
M src/kudu/master/sentry_authz_provider.cc
M src/kudu/master/sentry_authz_provider.h
M src/kudu/sentry/CMakeLists.txt
M src/kudu/sentry/sentry-test-base.h
M src/kudu/sentry/sentry_action.h
A src/kudu/sentry/sentry_authorizable_scope-test.cc
A src/kudu/sentry/sentry_authorizable_scope.cc
A src/kudu/sentry/sentry_authorizable_scope.h
M src/kudu/sentry/sentry_client-test.cc
M src/kudu/sentry/sentry_client.h
11 files changed, 508 insertions(+), 107 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/00/12500/4
--
To view, visit http://gerrit.cloudera.org:8080/12500
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I89437a04a4fa18e501d21c3abf5d66a2d22ce58a
Gerrit-Change-Number: 12500
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Hao Hao <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
